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

API style issues #1

Open
mzabaluev opened this issue Jan 28, 2021 · 0 comments
Open

API style issues #1

mzabaluev opened this issue Jan 28, 2021 · 0 comments

Comments

@mzabaluev
Copy link

Here's a list of observations on the API of this crate. I realise that some of these are probably caused by requirements of wasm-bindgen, but leaving this to consider possible ergonomic improvements, especially if the crate is meant to be used by Rust developers as well.

  • Most of the new_* constructors are clone-happy. In good Rust style these should not have reference parameters, but take values to move into the structures.
  • as_* methods (e.g. Label::as_int, Label::as_text) break the naming convention for conversion methods. These should perhaps be named like accessor methods (Label::int, Label::text).
  • The aforementioned accessor methods could be less clone-happy if they returned a reference to the inner object: Option<&str> instead of Option<String>, if only such signatures are supported by wasm-bindgen.
  • Rust naming style frowns upon all-caps components in CamelCase, so CoseSignature should be preferred over COSESignature.
  • COSESignatureOrArrCOSESignature* names are unwieldy. OneOrManyCoseSignatures?
  • The other_headers member of HeaderMap is public, leaking the unnecessary detail of it being implemented as a LinkedHashMap (from the linked-hash-map crate that is no longer actively maintained and has several possibly better alternatives).
  • It looks as though much of the library is dedicated to a partial implementation of the COSE spec and not CIP-0008 as its application. That might look good as a separate crate or at least a module? You could then have short names like cose::Signature, but then this Rusty style does not have an obvious mapping to Javascript bindings.
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

No branches or pull requests

1 participant