[java] adding values in enum in a new template version

[avaiss]

If a SBE template new version updates an enum by adding a value to it ; then the generated code for the previous version will throw an exception when trying to decode the enum.

For example:

 public static BlockTradeCode_enum get(final byte value)
    {
        switch (value)
        {
            case 66: return Block_Trade;
            case 78: return Regular_trade_or_Negotiated_deal;
            case 45: return Undefined;
        }

        if ((byte)0 == value)
        {
            return NULL_VAL;
        }

        throw new IllegalArgumentException("Unknown value: " + value);
    }

Is this by design ?

An alternative would be to add the UNKNOWN value for the enum and return it instead of throwing, so existing code won't break in that case.

[mjpt777]

Enum extension was not covered in the SBE Specification to the best of my knowledge. Maybe best to ask there.

https://github.com/FIXTradingCommunity/fix-simple-binary-encoding

[avaiss]

The case is not really specified, but this is documented here, letting the choice to the implementation.

https://github.com/FIXTradingCommunity/fix-simple-binary-encoding/blob/master/v1-0-RC4/doc/05SchemaExtensionMechanism.md#comaptibility-strategy

Also, an old decoder may encounter unexpected enumeration values. The application layer determines whether an unexpected value is a fatal error. Probably so for a required field since the business meaning is unknown, but it may choose to allow an unknown value of an optional field to pass through. For example, if OrdType value J="Market If Touched" is added to a schema, and the consumer does not recognize it, then the application returns an order rejection with reason "order type not supported", even if it does not know what "J" represents. Note that this is not strictly a versioning problem, however. This exception handling is indistinguishable from the case where "J" was never added to the enum but was simply sent in error.

By throwing, the decoder almost takes the decision for the application layer :-)

For a feed handler where the paradigm is almost everytime to switch on an updateType enum, an unknown value has exactly the same impact as an invalid one: ignore and continue.

I have no clean solution : my question is mostly about if this is an issue for which a patch optionaly generating an unknown enum value instead of throwing would be ok ; or if users should try/catch around the decoding of an enum and do whatever they want afterthat.

[da4089]

I was discussing this with @billsegall yesterday (in the context of his Go stubs implementation). I agree that at the application level, this will almost always end up falling through to the default case, where it'll be perhaps logged and otherwise ignored.

For languages without strong enum types, simply returning the value seemed like the best solution. If the language forces strong enums on you, then perhaps throwing is the best option?

[mjpt777]

It is an interesting point to ponder. We could support an UNKNOWN enum type when decoding. This would have the implication that the encoder would have to check for the UNKNOWN type and throw an exception if one is used, this adds code and an branch.

We could support a stub generation option for this type of code if enums are extended. Happy to hear views on this? @tmontgomery do you have a view?

My concern is the spec has no meta support for indicating in which version a enum value was added so it feels a bit dirty.

[billsegall]

I'm choosing to make the golang implementation structure based rather than flyweight and given the additional nature of the go language this means that any errors are returned during the decode as opposed to exceptions being raised in a getter.

As a result, I felt that returning the value as it came across the wire which would would then hit the default case in application code was an appropriate idiomatic mechanism of dealing with this.

Setting it to a defined NULL_VALUE would be an option but it seems like information loss and I'd rather put decision/control in the application programmer's hands. I can imagine cases where in a crunch this will lead to a much easier fix in a production system.

Other languages may have different idiomatic choices and that;s healthy. And of course this is only in the absence of specification. If it gets defined we can change it.

[mjpt777]

I've pushed a commit that provides this support as an option to the SbeTool and for the Java Generator. Be interesting to know @tmontgomery view on what is best for C++?

[da4089]

I think a similar issue arises for choices in a set, although the semantics of bit-flags makes returning false for unknown (as the current stubs do) more acceptable?

That said, I could see a situation where I'd prefer to get an "unknown" constant rather than false there too.

[avaiss]

@da4089 Choices in bitsets are not so much a problem at least when a new value is added. An old decoder should work fine even if it new choices were added in a younger version of the template. As there can't be an accessor to an yet unknown choice, there is nowhere to return unknown.

@mjpt777 nice ! I will try it tomorrow.

[da4089]

@starch yes, but with a newer decoder and an older peer, it can only get back false rather than a value indicating that the peer didn't provide a value.

I think this is less of a problem than with enumerations because the nature of a bitset is that false can be the default, but that's not always the right approach, and it'd be nice if when actingVersion < sinceVersion, access to a bit that's not supplied by the peer could be made to indicate that it wasn't provided.