cbor
cbor copied to clipboard
fxamacker
feature: Shorthand encmode/decode functions for predefined encoding options
Is your feature request related to a problem? Please describe.
Seeing a bunch of em, _ := cbor.CTAP2EncOptions().EncMode() is a bit of a code smell (at least to me)
Describe the solution you'd like
Given that these predefined options would ostensibly never error, provide similar predefined encoding/decoding modes that can be used inline à la json.NewEncoder(blah)....
Example:
cbor.CTAP2EncMode().Marshal(&obj)
Describe alternatives you've considered
- continuing as described above
- creating my own shorthand function in my library
Additional context n/a, this is just a suggested API improvement
@e3b0c442 thanks for opening this issue. It escalated quickly from 1 simple func to a full CTAP2 interface providing encoding & decoding funcs. :rofl:
Seeing a bunch of em, _ := cbor.CTAP2EncOptions().EncMode() is a bit of a code smell (at least to me)
More than 1 call to cbor.CTAP2EncOptions().EncMode() isn't needed. For best performance, a mode should be created just once and reused.
If you write a func that is called more than once, that func should reuse a mode rather than call EncMode() itself.
I should note I'm happy to submit a PR for this if accepted.
A CTAP2 interface requires more time & effort than what you initially proposed, so I think @fxamacker will code it (if approved). It would involve both encoding and decoding.
Details
Separation of options and modes allows sanity checks and translation into different data structures (to speed up encoding or decoding) so that overhead shouldn't be repeated.
Ideally, mode(s) should be created during startup and preferably before init() runs. Any helper functions requiring modes would simply reuse the modes created during startup.
I agree the API should make CTAP2 easier. CTAP2 is special case. In part because other predefined options are typically starting points for modification.
@fxamacker please consider exporting a CTAP2 interface that provides these:
Marshal, Unmarshal, NewEncoder, NewDecoder.
- they should be ready to use before init() runs
- be callable as
cbor.CTAP2.Marshal(v),cbor.CTAP2.Unmarshal(b, &v), etc.
CTAP2 Canonical CBOR should be reviewed to make sure decoding (not just encoding) is in compliance with any extra limitations imposed by CTAP2. Use io.LimitReader, etc. if needed.
It escalated quickly from 1 simple func to a full CTAP2 interface providing encoding & decoding funcs.
Would it though?
The OP-proposed cbor.CTAP2EncMode() could essentially just return a ctap2EncMode in the same way that defaultEncMode is used already. It's not a different type (thus not a different interface implementation), but rather just another unexported global variable.
Whether or not the extra globals are warranted is an open question.
This could also be handled with a must style function which can be documented as guaranteed to succeed for any of the bundled EncOptions constructors, i.e. must(cbor.CTAP2EncOptions().EncMode()) would not panic.
