nikita-volkov / typeclasses

Explicit typeclasses for Elm
https://package.elm-lang.org/packages/nikita-volkov/typeclasses/latest/
MIT License
54 stars 3 forks source link

Expose general-purpose Int tumbling function #2

Closed mitchellwrosen closed 5 years ago

mitchellwrosen commented 5 years ago

hashable defines this:

-- | Combine two given hash values.  'combine' has zero as a left
-- identity.
combine :: Int -> Int -> Int
combine h1 h2 = (h1 * 16777619) `xor` h2

Could this be added to Typeclasses.Classes.Hashing? It is a rather convenient way of tumbling ints together :)

mitchellwrosen commented 5 years ago

Oh, I see that this is already used in Int's instance. So I think hashWithSalt is totally sufficient. Shall I close?

mitchellwrosen commented 5 years ago

Actually, I've found it convenient to use an alias for combine with the arguments flipped. This lets you start with a salt and mix in ints in postfix-application style as in:

initialSalt
  |> tumble thing1
  |> tumble thing2

(where tumble = flip combine). This can be done with Hashing.int.hashWithSalt but it's more awkward:

initialSalt
  |> (\s -> Hashing.int.hashWithSalt s thing1)
  |> (\s -> Hashing.int.hashWithSalt s thing2)
nikita-volkov commented 5 years ago

Such function is of lower-level of abstraction, so it would be incorrect to expose it as part of the API of the typeclass.

It also seems likely that you're doing something that can be provided for with a higher-level API. What are you doing with this exactly?

mitchellwrosen commented 5 years ago

I'm writing Hashing instances for ADTs with the simple algorithm of:

nikita-volkov commented 5 years ago

Okay. So it looks like you need something like the choose function on it. Would that suffice?

nikita-volkov commented 5 years ago

How about this addition? https://github.com/nikita-volkov/typeclasses/commit/751b149ee24c65671bbd23a3aa61832b3b9d2140

mitchellwrosen commented 5 years ago

Looks alright, I think you have indeed found the right abstraction for this. I've never quite figured out how to make that hierarchy work as nicely as Applicative seems to, though. In your example you have a three-constructor ADT, which requires two nested eithers... but in my case, I have like 19 constructors. I can probably figure something out if I play with it, though. I just want my code to grow vertically, like

stuff
  |> stuff
  |> stuff

or similar, rather than

x
  |> andThen
    (\y ->
      z
        |> andThen
          (\q ->
            ...))

as often seems to happen in Elm, esp. with elm-format turned on.

nikita-volkov commented 5 years ago

The only better way I know is to have sum-types for arbitrary arities, which would let us have an API like this:

sum2 : Hashing a -> Hashing b -> Hashing (Sum2 a b)
sum3 : Hashing a -> Hashing b -> Hashing c -> Hashing (Sum3 a b c)

Essentially it is the same as what tuples achieve for product-types. Unfortunately there's no library in Elm exposing these and I doubt one would get adopted much, so I don't dig into that direction.

Anyway, I've just released the either addition. Would you consider this issue closed?

mitchellwrosen commented 5 years ago

Yes I would consider the issue closed, thanks for the help!

nikita-volkov commented 5 years ago

In case someone is looking for more examples, check out the issue #4.