Jump to content

Inheritance in this situation feels a bit icky...


Recommended Posts

Back with another "there-must-be-a-better-way" question. I have a situation that may work, but feels icky.

I have an incoming message that has an array of u8 data. A subset of this data is parsed to determined the "type" of the message. There are 4 different types of messages so I have a class for each type, and a factory that creates the proper one based on the type. Then I have a method for each type that futher parses the u8 array (the array is parsed differently based on message type). This gives me a bit more information, and I finally know the "final" type of message so I can generate that class.

Basically I have a hierarchy that looks like this

(ugh didn't format correctly but you get the idea)

base message

/ / \ \

type1 type2 type3 type4

/ | \

finalsubclass

The issue is, because I find out a little more information with each step of parsing the u8 array, which tells me how to parse the next step I keep having to "downcast" my class which means continually trying to preserve data that already exists in the parent class. I have seen the methods mentioned here but it gets really hairy with multiple levels of children. From purely a cosmetic standpoint, the levels of inheritance seem to make sense because they match the protocol of the messages I am receiving (there are multiple messages that can come in, these can be of different types (this is the first child class), and those types have children (grandchildren class)).

I'm just looking for suggestions that may make my life easier.

Edited by for(imstuck)
Link to comment

I assume you have no control over the format of the message? Objects are best flattened/unflattened upwards from child to parent, which means the child information should precede parent information in the message. Your messages are the opposite. I suspect you can still parse it without “downcasting”. You’d have to generate a “type” object to call a parse method, but rather than set the private data of the parent type, the parse method would call further parse methods on the remaining bytes to actually return the subclass. So the parse method called on the “type” object would actually return a different object of the final subclass. Does that make sense? It’s a bit like the onion skin of recursion; instead of calling methods one after the other, you call them one inside the other, going all the way down to the final subclass, and then rising back up through the inheritance hierarchy setting the private data of each level.

— James

Link to comment

I assume you have no control over the format of the message?

— James

Correct, I actually meant to put that in there. I was thinking, "it would be nice if I could change the message order" while I was writing my post. It definitely makes sense, but implementing it could be a different story. I'll see where I can get on my own but may come back for suggestions and clarifications.

Link to comment

Correct, I actually meant to put that in there. I was thinking, "it would be nice if I could change the message order" while I was writing my post. It definitely makes sense, but implementing it could be a different story. I'll see where I can get on my own but may come back for suggestions and clarifications.

In my cross post to here does the response I got seem valid? I am still questioning it.

Link to comment

I have an incoming message that has an array of u8 data. A subset of this data is parsed to determined the "type" of the message. There are 4 different types of messages so I have a class for each type, and a factory that creates the proper one based on the type. Then I have a method for each type that futher parses the u8 array (the array is parsed differently based on message type). This gives me a bit more information, and I finally know the "final" type of message so I can generate that class.

Sounds to me like a classic example of serialization. The caveat being you might be forcing a hierarchy that is different (or possibly didn't exist) on the other end.

Why not just extend the factory up through the various layers?

It probably goes without saying, but I'll be explicit: the factory methods are static (not dynamic dispatchers, probably don't even have a class terminal). BaseMessage.Create() reads what it needs to in order to delegate to any of the TypeN.Create() methods. Similarly the TypeN.Create() method reads what it needs to, and delegates another level to the appropriate FinalSubclassN.Create() method. It's only however many levels deep into the Create() methods when the class is fully resolved that the appropriate type is determined and returned. Any dynamic dispatching happens after the create factory stack has sorted out the type.

Err yeah, looks like PiDi had a similar recommendation.

And for the record, when I said "probably don't even have a class terminal", I meant input terminal.

Link to comment

I'm also kind of on board. Certainly for the first level there seems to be little benefit and much complexity added. Another way you could read this stage is that the type here is a property of the first message rather than a unique class of message.

For the second stage it seems there could be more to gain but not huge amounts. You would add some scalability at your read message stage but in just that area for the extra complexity that propagates throughout the code. (I would get frustrated if I had to dig through two layers of delegates each time I wanted to read the code that was actually running)

My 2c, I think the suggestions made look good if you wanted to dispatch it but I think (with the limited scope of what you have described here) it could be unnecessarily complex

Link to comment

My 2c, I think the suggestions made look good if you wanted to dispatch it but I think (with the limited scope of what you have described here) it could be unnecessarily complex

^^^ This! I have an absurd inheritance heirarchy trying to encapsulate the specific functions for each "step" in the parsing into their particular classes within the heirarchy. That is what really brought me to ask this question in the first place. Maybe I'll just make a message parser class and stick all methods for parsing in there, and it can return the correct message with it's private data populated, allowing me to scrap some of the parent message classes (which are really there only to hold their parsing functions and a few properties that apply to all child messages).

Link to comment

So, just to come full circle on this post, what I have ended up doing is the following:

I realized, as my message comes in it consists of two things, a numeric value and an array of data which goes into a class, call it BaseMessage. I then had type n message inherit from this class, and grandchildren messages inherit from that. I thought about it using all of your suggestions, and I came to realize, the first level of inheritance was not necessary. What really is happening is I am using the data from the message received to create a new class, and this new class doesn't care about the byte array of data as a byte array. Therefore, it doesn't need it in order to maintain state and as such doesn't need to inherit from the BaseMessage class. It only cares about what that data represents once it's been taken out of its byte array form (temperatures, pressures etc). So, when I create my classes, the "constructor" now takes the BaseMessage as an input. My new class is being created from the data in BaseMessage, but is really not a base message. Taking this step begins to clear things up quite nicely. Any comments on this are appreciated, but it seems to work quite well. Thanks for all the suggestions.

Edited by for(imstuck)
Link to comment

Thought I might just mention the player-role pattern for this application. The player in this case would be the BaseMessage, and each Role (Role1, Role2, etc.) would be a different message type.

http://t0.gstatic.com/images?q=tbn:ANd9GcRNqGbKVQCBxYU8kakLJemmOrKhD4ezy1FhFrvdAqgdu3sMUrsrVNBr3SSX

Just a thought I wanted to throw out there..

Link to comment

Shut down your computer and head home for the day because the world is ending... I agree with ShaunR on this one. Collect the data as you read, and when you have read all the pieces to know exactly your final type of object, then and only then instantiate your object. You'll probably end up with some sort of "variant with named attributes hanging off of it" to collect as you read and then you'll evaluate the collected attributes and make it into an object.

Link to comment

Shut down your computer and head home for the day because the world is ending... I agree with ShaunR on this one. Collect the data as you read, and when you have read all the pieces to know exactly your final type of object, then and only then instantiate your object.

:o

You don't have to tell me twice. If my boss asks where I'm going, I'll just say Stephen told me to go home because he agreed with ShaunR. I do already read all the data at once (comes out of a CAN bus so it's just a single read that returns all message data), it's just parsed differently depending on the type of message. I have kind of been toying with different ideas and what I ended up doing was creating a "message parser" class, which has children parsers. I have a method that checks the class type, then instantiates the correct parser. This parser has methods that know how to act on the message of that particular type. It gets all the data, like suggested above, then sticks it into a class. This way I keep all the message parsing encapsulated in its own class, and can reuse the parser class throughout the different modules in my code where messages are received and needed to be managed this same way. I may be off base but it seems to work, keep things more loosely coupled, and be reusable. I can't be way off base then, I assume! But, if down the road it causes problems and I need to refactor, I'll chalk one point up to lessons learned.

Edited by for(imstuck)
Link to comment

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Unfortunately, your content contains terms that we do not allow. Please edit your content to remove the highlighted words below.
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...

Important Information

By using this site, you agree to our Terms of Use.