0
\$\begingroup\$

My application accepts different messages. A message is described by its schema.

enum MessageFormat: represents all kinds of messages. (XML, JSON, custom)

interface Message: represents an abstract Java Object for any kind of message.

interface Schema: represents an abstract Java Object for any kind of message descriptor. Contains MessageFormat.

enum SchemaFormat: represents all kinds of schemas. (XSD, JSON Schema, XML respecively for each MessageFormat)

interface MessageParser: has a method Message parse(String, Schema). There is one concrete for each MessageFormat.

interface SchemaParser: has a method Schema parse(String). There is one concrete for each SchemaFormat.

At start-up, all of the supported schemas are loaded and parsed one time each and kept inside the memory as concrete Schema objects. When a message comes, we pass the message with its schema to the appropriate parser. It would be optimal to also have each appropriate parser instance inside memory (schema - parser, one-to-one). What is the best way to achieve this?

My current layout is to have a Strategy pattern switching the static parsers by asking a Factory pattern for the reference at the static parser that is inside that factory (as well as all of the parsers, for schemas and for messages, all are static).

Strategy:

public class Deserializer {

    private static MessageParser messageParser;
    private static SchemaParser  schemaParser;

    private Deserializer() {} // Imitating a static class

    public static Schema deserializeSchema(String schema,
                                           SchemaFormat schemaFormat
                                          ) {

        setSchemaParser(schemaFormat); // Strategy pattern
        return schemaParser.parse(schema);
    }

    public static Message deserializeMessage(String msg,
                                             Schema schema
                                            ) {

        setMessageParser(schema.getMessageFormat()); // Strategy pattern
        return messageParser.parse(msg, schema);
    }

    private static void setMessageParser(MessageFormat msgFormat) { // Strategy pattern

        messageParser = ParserFactory.getMessageParser(msgFormat); // Factory pattern
    }

    private static void setSchemaParser(SchemaFormat configFileFormat) { // Strategy pattern

        schemaParser = ParserFactory.getSchemaParser(configFileFormat); // Factory pattern
    }
}

Factory:

public class ParserFactory {

    private static final XMLSchemaParser                XML_SCHEMA_PARSER                  = new XMLSchemaParser();
    private static final FixedLengthStringMessageParser FIXED_LENGTH_STRING_MESSAGE_PARSER = new FixedLengthStringMessageParser();

    private ParserFactory() {} // imitating a static class

    public static MessageParser getMessageParser(MessageFormat msgFormat) {

        switch (msgFormat) {
            case FIXED_LENGTH_STRING:
                return getFixedLengthStringMessageParserInstance();
            default:
                return null;
        }
    }

    public static SchemaParser getSchemaParser(SchemaFormat schemaFormat) {

        switch (schemaFormat) {
            case XML:
                return getXmlSchemaParser();
            default:
                return null;
        }
    }

    private static XMLSchemaParser getXmlSchemaParser() {
        return XML_SCHEMA_PARSER;
    }

    private static FixedLengthStringMessageParser getFixedLengthStringMessageParserInstance() {
        return FIXED_LENGTH_STRING_MESSAGE_PARSER;
    }
}

I've been suggested to wrap all of the parsers inside the enums like so:

enum SchemaFormat {
    XML(new XMLSchemaParser());

    final SchemaParser schemaParser;

    SchemaFormat(SchemaParser schemaParser) {
        this.schemaParser = schemaParser;
    }
}

And then use them like so:

public static Schema deserializeSchema(String schema, SchemaFormat schemaFormat) {
    return schemaFormat.schemaParser.parse(schema);
}

That way I can just completely delete the Deserializer and the ParserFactory classes.

If they are present, however, each Parser in lazy-initialized and as soon as it's initialized, it's kept in the memory for the rest of the time.

If I have all of the Parsers inside the enum then for each message an object gets declared and then discarded. But what happens with the performance when the application obtains 150 messages of 5 different types within a short period of time? I'm concerned that it would be really slow and the memory will bloat until the garbage collector comes and cleans it.

I also don't have the way to load the schemas at the start-up, so I may choose the option with putting the SchemaParsers inside the SchemaFormat just for the reason I will parse them once. But several data providers may choose the XML schemas, so, I that case I will instantiate the XML parser twice but that isn't a big deal, because there will not be more than 10 data providers.

\$\endgroup\$
2
\$\begingroup\$

Multithreading

You're making multithreading really hard on yourself by using static fields in this class. Consider the following:

public static Schema deserializeSchema(String schema, SchemaFormat format) {
    return ParserFactory.getSchemaParser(format).parse(schema);
}

public static Message deserializeMessage(string msg, Schema schema) {
    return ParserFactory.getMessageParser(schema.getMessageFormat()).parse(msg);
}

This reduces all the points of failure in the Deserializer from Multithreading to naught. It's also significantly cleaner from a responsibility viewpoint. Assuming you want to get this a bit less dense, you can extract local variables.

Factory Responsibilites

It's generally more conventional to name the factory after the kind of objects it creates. Consider separating these two object types into factories of their own:

public final class MessageParserFactory {
    private static final MessageParser FIXED_LENGTH = new FixedLengthStringMessageParser();

    private MessageParserFactory() {}

    public static MessageParser getMessageParser(MessageFormat msgFormat) {
        switch (msgFormat) {
           case FIXED_LENGTH_STRING:
                return FIXED_LENGTH;
           default:
                return null;
        }
    }
}
\$\endgroup\$
  • \$\begingroup\$ Thanks! What do you think about putting everything inside the enums? \$\endgroup\$ – Sam Aug 4 '17 at 12:14
  • \$\begingroup\$ an abuse of enums ... but sure go ahead, after all java explicitly does not have actual enums ... \$\endgroup\$ – Vogel612 Aug 4 '17 at 12:50
  • \$\begingroup\$ @Vogel612 And why, exactly, is declaring a final field in an enum with a (presumably) immutable type an abuse of enums? \$\endgroup\$ – Stingy Aug 4 '17 at 15:05
  • \$\begingroup\$ @Stingy because enums are not supposed to be classes... \$\endgroup\$ – Vogel612 Aug 4 '17 at 15:07
  • \$\begingroup\$ @Vogel612 Do you mean that an enum should not have fields and methods in the first place? \$\endgroup\$ – Stingy Aug 4 '17 at 15:09

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.