gleam-lang / crypto

⛓️ Crypto functionality for Gleam applications
https://hexdocs.pm/gleam_crypto/
Apache License 2.0
35 stars 10 forks source link

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

Closed Xetera closed 5 months ago

Xetera commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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

lpil commented 5 months ago

Great news, thank you