Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

for In the project I am working on we send messages about widgets over message queues, serializing them onto the queues as XML. The XML schema contains tags for properties that are common to all types of these widget messages, such as the widget type, command name and destination. It can also contain an arbitrarily sized list of key-value pairs to allow storage of properties that are only relevant for a specific type of widget message. The WidgetMessage class encapsulates this data and the WidgetMessageXmlWriter and WidgetMessageXmlReader classes provide serialization to and from the XML.

I have made some classes that encapsulate specific messages, for example a FooPlaySoundMessage for a 'Foo' widget or a BarSetLightPatternMessage for a 'Bar' widget. They each have a ToWidgetMessage instance method and a FromWidgetMessage static method for converting to and from the WidgetMessage class. Each family of messages inherits from an abstract class for that widget type, e.g. FooMessage and BarMessage, which in turn inherits from the WidgetMessageMapping class; this stores the common message properties and the protected methods used by subclasses for conversion. None of these classes inherit from WidgetMessage since I don't want them to inherit its key-value collection property and associated methods, hence the need for conversion rather than simple casting.

I like the simplicity of my API (e.g. FooPlaySoundMessage msg = FooPlaySoundMessage.fromWidgetMessage(widgetMessage)), but the fact I am having to use protected methods in a base class to share functionality, and static methods to expose it, makes me wonder if there ought to be a separate class or two involved here (similar to WidgetMessageXmlWriter and WidgetMessageXmlReader). On the other hand, I thought that part of the point of OOP is to group data and methods together and so avoid "dumb data objects".

So, do I have the right idea by adding conversion methods to my data objects, or should that functionality be extracted out into another class?

Update:

I think that in all the detail above of my current attempt at a design I didn't explain clearly enough the problem I am trying to solve.

In summary, I have a "generic" DTO class that has some strongly typed properties and a collection of key-value pairs to store other custom data. I want to have some specialized DTO classes for each set of custom data that store all the same data as the generic DTO, except with the key-value pairs replaced by strongly-typed properties. What is the best design for converting between these two types of DTO?

share|improve this question
    
Sounds like Decorator pattern? - en.wikipedia.org/wiki/Decorator_pattern –  Stevo Feb 27 '13 at 16:37
    
Second that. I think you're fine. –  Stu Mar 1 '13 at 19:17
1  
Also, as a random aside: if it works for you, it's in. –  Stu Mar 1 '13 at 19:18
    
What does the message do instead of just being the message? –  Piotr Gwiazda Mar 13 '13 at 20:53
    
The sole responsibility of these classes is to be converted to and from the WidgetMessage class. There is a 'Widget Comms' project that consumes WidgetMessages and talks to the widgets; I have not added such communication functionality to these WidgetMessageMapping classes because only the Widget Comms project should know how to talk to the widgets, whereas the messages can be created in various different parts of the application. The WidgetMessageMappings are there simply to avoid message parsing and creation code being spread across the layers of the application. –  Robert Johnson Mar 14 '13 at 8:25
add comment

1 Answer 1

If FooPlaySoundMessage knows how to play sound and also how to map itself to a message queue format you could say the class has more then one responsibility. If it directly delegates the actual sound playing to a different class your FooPlaySoundMessage is basically a data transfer object. Putting common mappings in a shared base class seems fine to me in that case.

I would probably separate it out though. Data transfer objects typically have little to no code, you could see FooPlaySoundMessage as one.

At some point you may have to transmit the same data by some other means or some other format (json perhaps?). You could YAGNI it for now and separate it out then.

I'd probably make some message handler class that parses out the common parts, detects the message type and then delegates to a specific parser e.g. FooPlaySoundXmlMessageParser. When in doubt favor composition over inheritance.

share|improve this answer
    
It is only the WidgetMessage that gets converted to and from XML - the specialised DTOs will only ever be converted to and from WidgetMessages. In my current design they are still called 'messages', even though it is only the WidgetMessage that gets put onto a queue, so I can understand your confusion. I've updated my question to better explain the problem I'm trying to solve. –  Robert Johnson Mar 19 '13 at 13:45
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.