1

I have a Message class which can contain multiple types of payloads (or sometimes no payload), each derived from a common Payload class. However, this becomes problematic because the Message class wants to know about the Payload subclasses for various reasons such as:

  • Checking for Message equality

    if (message parts besides the payload are equal) {
        switch(type) {
            case Payload::Type::RESPONSE:
                return *static_cast<ResponsePayload*>(payload.get())
                    == *static_cast<ResponsePayload*>(o.payload.get());
                break;
    
            case Payload::Type::SETUP:
                return *static_cast<SetupPayload*>(payload.get())
                    == *static_cast<SetupPayload*>(o.payload.get());
                break;
    
            ...
        }
    }
    
  • Deserializing (since the deserialization methods are static because the payloads are immutable)

    switch(type) {
        case Payload::Type::RESPONSE:
            load = ResponsePayload::fromJSON(payloadValue);
            break;
    
        case Payload::Type::SETUP:
            load = SetupPayload::fromJSON(payloadValue);
            break;
    
        ...
    
        case Payload::Type::START:
        case Payload::Type::STOP:
        case ...:
            break; // Load stays null
    
        default:
            THROW(Exception, "Error in program logic: we forgot to parse some payload");
    }
    
  • Ensuring a Payload is attached to a Message while constructing it:

    switch(type) {
        case Payload::Type::RESPONSE:
        case Payload::Type::SETUP:
        case ...:
            ENFORCE(IOException, payload != nullptr, "For this message type, a payload is required.");
            break;
    
        case Payload::Type::START:
        case Payload::Type::STOP:
        case ...:
            ENFORCE(IOException, payload == nullptr, "For this message type, the payload should be null");
            break;
    
        default:
            THROW(Exception, "Error in program logic: we forgot to handle some payload");
    }
    

Alarm bells are going are going off in my head - this violates SOLID like there's no tomorrow and obviously doesn't scale well since I have to add case statements every time I add a new payload. Is there a cleaner approach I could take?

  • Does it makes sense, in your design, for a basic payload object to know about the message object? – Crono Mar 25 '14 at 20:37
3

Alarm bells should be going off. But some top of my head initial suggestions (meaning I didn't take the time to come up with the best solution, just a much better one than you've got).

1 - Why is there a separate Message and Payload class, aren't they the same thing?

2 - Include the equality check in the subclasses where they take a payload instance as the parameter. Subclasses only know about their own type. If you pass a payload that isn't the right type then the type conversion will fail and you know they aren't equal.

3 - Create a Payload factory instead of creating all the various types in the base message class.

4 - Subclass the Message class if it requires parameters to be built properly.

  • 1
    Yes to all of this. OP should also have the payloads handle more logic. – Zeroth Mar 25 '14 at 21:58
  • Thanks for the advice! I'm removing the Payload class and just subclassing Message, and things are already looking a lot cleaner. – Matt Kline Mar 26 '14 at 16:29

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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