0

I looked at the answers for this question: Design pattern for handling multiple message types

and I'm in a similar boat. The difference I have is, I'm using an existing protobuf schema that I cannot change. The protobuf schema does have the messageType property. The generated code looks like

TradeMessage.parseFrom(byte[] bytes)
OtherMessage.parseFrom(byte[] bytes)
AnotherMessage.parseFrom(byte[] bytes)

So right now I have a factory pattern that when a message comes in in the receiver 

MessageReceiver.java

Object parser = messageParserFactory.getParser(messageType);

Get the type of parser

MessageParserFactory.java

public MessageParser getParser(int messageType) {
    if (messageType = Constants.TRADE_MESSAGE) {
        return new TradeParser();
    } else if (messageType = Constants.OTHER_MESSAGE) {
        return new OtherParser();
    }
    return null;
}

Basically repeated work for all the different message types that essentially just wrap the generated parseFrom method.

public interface MessageParser {
    void doParse(byte[] bytes);
}

TradeParser.java
public void doParse(byte[] bytes) {
    TradeParser.parseFrom(bytes);
}

OtherParser.java
public void doParse(byte[] bytes) {
    OtherParser.parseFrom(bytes);
}

AnotherParser.java
public void doParse(byte[] bytes) {
    AnotherParser.parseFrom(bytes);
}

It works but is there a better way since basically all the parsers I create for each message type do the exact same thing and just call parseFrom.

Andy Turner
  • 137,514
  • 11
  • 162
  • 243
Crystal
  • 28,460
  • 62
  • 219
  • 393
  • Do you need the `MessageParser` at all? Why not just return the parsed instance? – Andy Turner Jul 16 '17 at 21:58
  • 1
    `messageType =` should be `messageType ==`. I take it this is just kinda pseudocode? – Michael Jul 16 '17 at 22:00
  • Also, don't return `null` if it's an unsupported message type, throw an exception. – Michael Jul 16 '17 at 22:01
  • @AndyTurner no I don't need the message parser at all. It's only to unclutter the main entry point for clients. In reality I could just have a switch statement for the message types and just return the parsed object. – Crystal Jul 16 '17 at 22:01
  • @Michael ya just all pseudocode. Thx. – Crystal Jul 16 '17 at 22:02
  • @AndyTurner is it cleaner to have a big switch statement like that since the message parser adds no value? – Crystal Jul 16 '17 at 22:03

1 Answers1

0

Will you ever have a huge number of message types? Are your messages complex?

In any case, I see why you may think it's unnecessary as you're just wrapping your parseFrom method at the moment. It could become interesting if your parsing become complex rather than just a call to the parseFrom method.

For the moment, would it not be best for you to store a key-value array with the messageType as key and the Class as value and then use reflection to instantiate your message?

I think this could be rather clean as you could define your array in a context/config file.

Maaaatt
  • 429
  • 4
  • 14
  • The number of message types will be less than 20. I don't for see much logic in the parsers except parsing the messages. The logic is in another area of the app. – Crystal Jul 16 '17 at 21:47
  • How would I do that with reflection? Is there any performance cost since each time a message comes in from a client, it would have to go down that right to get the messageParser. – Crystal Jul 16 '17 at 21:52
  • Yes, there is a significant performance cost to instantiate via reflection. However, I assumed that your parseFrom methods were probably static and I believe that calls to static methods are less expensive. Wouldn't be able to remember the source so take this statement with caution or even better, test it! In logic, you would have to look through your array, get the class associated with your message type and then make a call to your parseFrom with reflection. It will be something like: Method m = clazz.getMethod("name", argTypes); Object o = method.invoke(null, args); – Maaaatt Jul 16 '17 at 22:07