Skip to content

Conversation

@swmorrow
Copy link

The library as it is allows for custom encoding using tags, which makes it good for places where data is passed between projects of an internal project. However, there is no provided mechanism to decode tags, meaning users have to write their own functions. These functions must be recursive if iteration is required (for example, if one decides to add a custom tag for tuples).

These changes allow for a keyword argument to be passed in containing a decoder function, facilitating decoding custom tags. Examples can be found in updated documentation. Furthermore, this keyword option framework allows for further implementation of different keywords corresponding to custom input decoders in the future.

@tomciopp
Copy link
Collaborator

tomciopp commented Jul 13, 2022

@swmorrow Thanks for the PR, this is just acknowledging that I have received it. I will take some time later tomorrow to review it and let you know what I think.

@tomciopp
Copy link
Collaborator

Ok so here are my thoughts. If we are going to add this feature, I think we should prefer to have multiple tag_decoders instead of just a single decoder. I think it would be easier to extend your codebase piece by piece instead of having a single object that has to handle every single case. As written I think it could be a bit brittle and difficult to understand exactly what is happening.

This api seems more natural to me.

CBOR.decode(bin_tuple, tag_decoders: [AtomDecoder, TupleDecoder, MyCustomDecoder])

Then each of your custom decoders should implement a behavior (e.g. CustomTagDecoder) that has two methods that check if it should decode the tag and handles decoding the tag. If none of the tag_decoders match, we can then return the tag as is.

LMKWYT.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants