Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve support for debugging by throwing on missing ENUMs. #583

Open
ams-tschoening opened this issue May 24, 2019 · 0 comments · May be fixed by kaitai-io/kaitai_struct_compiler#288
Open

Comments

@ams-tschoening
Copy link

The problem.

At least in Java, mapping decimal values of some stream to some ENUM is done by manually creating a hash map of decimals to ENUMs and accessing that map, like in the following example:

    private static final Map<Long, ValueType> byId = new HashMap<Long, ValueType>(41);
    static {
        for (ValueType e : ValueType.values())
            byId.put(e.id(), e);
    }
    public static ValueType byId(long id) { return byId.get(id); }

What happens if the given decimal can't be found? null is returned, leading to NullPointerException in code accessing getters providing the ENUMs.

I have a lot of ENUMs which need to support only few values at the beginning and are extended as needed, so the exception isn't that wrong at all, because it tells me that I need to look at the parser to see if it's time to extend the ENUM once more. The problem is that the NPE doesn't tell me anything about which decimal was in the stream and stuff.

Possible solutions.

I only see two useful solutions, either providing a fallback value for any unknown decimal or simply throwing an exception including the unknown value. Some fallback would be in line with switch-on which supports one as well, the exception OTOH would be more what I really need.

In my opinion, mapping ENUMs is a good candidate for validations/assertions (#435) and something like UnexpectedDataError in the long term. Especially because of proposed improved debugging using positional infos in exceptions (#581), throwing an exception at all in combination with positions within the stream would make recognizing the missing ENUM and deciding what to do a lot easier.

Place mapping into runtime?

How about making ENUM-mapping itself part of the runtime? It's always the same for all of those besides that the type is different each time. This way the compiler would need to create less code and one could use all validations/assertions/exceptions available in the runtime in future.

Any thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants