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

Make the API more generic/trait-aware #29

Open
UARTman opened this issue Nov 6, 2022 · 7 comments
Open

Make the API more generic/trait-aware #29

UARTman opened this issue Nov 6, 2022 · 7 comments
Labels
bug Something isn't working

Comments

@UARTman
Copy link
Contributor

UARTman commented Nov 6, 2022

Right now Hex API is written without much regard for traits. For example, it has multiple from_ methods that use the same code relying on From trait, as well as a from_str method that conflicts with the FromStr trait.

These can be either replaced or augmented with code that's generic over traits. This will make Hex a bit more capable and idiomatic.

Then, if making Hex more generic/trait-aware is a success, we can do the same to other types where necessary.

@yegor256
Copy link
Member

yegor256 commented Nov 6, 2022

@UARTman good point, let's do it

@yegor256 yegor256 added the bug Something isn't working label Nov 6, 2022
@yegor256
Copy link
Member

@UARTman any progress here?

@UARTman
Copy link
Contributor Author

UARTman commented Nov 22, 2022

@yegor256 Thanks for reminding me! I'll draft the PR with the changes I've been able to make so far today evening or tomorrow.

UARTman added a commit to UARTman/sodg that referenced this issue Nov 23, 2022
UARTman added a commit to UARTman/sodg that referenced this issue Nov 23, 2022
UARTman added a commit to UARTman/sodg that referenced this issue Nov 23, 2022
Implemented `Eq` trait, designating that every two `Hex`es can be compared.
Removed a string equality hack from `PartialEq` implementation, removing unnecessary allocations.
@UARTman
Copy link
Contributor Author

UARTman commented Nov 23, 2022

@yegor256 I've done the first PR. However, there's still some ideas I'd like to look into, like a proper Into implementation and removing from_str_bytes and from_string_bytes entirely in favor of From<&str>.

@UARTman
Copy link
Contributor Author

UARTman commented Nov 23, 2022

@yegor256 Most of the other code wasn't as obviously in need of trait implementations, since most of our types aren't thin wrappers over bytes. However, I'll take a closer look at whether there are places that will benefit from using slices or borrows.

Also, a question: should I implement more Froms for, say, i32 and f32 or will only i64, f64 and bool be stored?

@yegor256
Copy link
Member

@UARTman yes, let's handle i32 and f32 too

@yegor256
Copy link
Member

@UARTman what's up with this job, not yet complete?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants