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

Hash function parameter order is not friendly for usage in pipes #6

Closed
Xetera opened this issue Mar 24, 2024 · 3 comments
Closed

Hash function parameter order is not friendly for usage in pipes #6

Xetera opened this issue Mar 24, 2024 · 3 comments

Comments

@Xetera
Copy link

Xetera commented Mar 24, 2024

The current implementation is this:

pub fn hash(a: HashAlgorithm, b: BitArray) -> BitArray

I can't imagine ever calling this function this way

get_dynamic_hash_algorithm()
|> crypto.hash(<<1,2,3,4>>)

it would be way more helpful if it had this signature

pub fn hash(a: BitArray, b: HashAlgorithm) -> BitArray

so it could be called like this instead

extract_bencode(info_field)
|> crypto.hash(crypto.Sha1)

I'm a little bit skeptical about how this would be implemented though, given that it's a pretty major breaking change. But hey a major release doesn't mean nothing can ever be broken right?

I was sent here by tynan on Discord

@lpil
Copy link
Member

lpil commented Mar 25, 2024

Could be good, but it would be a breaking change. That is rather annoying. Is it work it to avoid writing one _,?

extract_bencode(info_field)
|> crypto.hash(crypto.Sha1, _)

@Xetera
Copy link
Author

Xetera commented Mar 25, 2024

I didn't even know argument holes was a thing until now. Seems like a much better compromise than breaking changes in the API. That solves my problem, thanks

@Xetera Xetera closed this as completed Mar 25, 2024
@lpil
Copy link
Member

lpil commented Mar 25, 2024

Great news, thank you

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

2 participants