purescript-contrib/purescript-uint

UInt instances?

Opened this issue · 8 comments

maxdeviant commented

Looks like there was some previous discussion in #7 on this.

From what I can tell, the outcome of that issue was the addition of an Arbitrary instance in d3a6424.

The Arbitrary instance was then removed in 6820b50 when the QuickCheck dependencies were moved into the test dependencies.

Looking at the purescript-uint-instances package, it adds the following instances that we do not currently have:

  • 👍1
chtenb commented

I came here because I was looking for an Arbitrary instance for UInt. Is there a reason why we would not want to expose the Arbitrary instance in the API?

garyb commented

I think providing something in terms of MonadGen would be preferable since that has a much lower dependency overhead.

jamesdbrock commented

I would love a MonadGen PR @chtenb ... or Arbitrary, or whatever you think is best

chtenb commented

Right, so if MonadGen is preferred then we would have something like

chooseUInt :: forall gen. MonadGen gen => UInt -> UInt -> gen UInt
chooseUInt a b = fromInt $ chooseInt (toInt a) (toInt b)

but we're using the chooseInt from quickCheck here, so that doesn't solve the dependency overhead.

Wouldn't it perhaps be better to add this to the quickCheck library instead then? The quickcheck lib would gain uint as a dependency, but that's a very small library. Another advantage would be that we could add Arbitrary while we're at it.

jamesdbrock commented

You don't need the quickcheck dependency, you can just use the chooseInt provided by the MonadGen. See for example https://pursuit.purescript.org/packages/purescript-float32/2.0.0/docs/Data.Float32.Gen

chtenb commented

You're right @jamesdbrock
So in that case I'd say we add the chooseUInt based on MonadGen to this package and then also provide the arbitrary instance in the quickcheck package, if the folks there are okay with that.

chtenb commented

We seem to have overlooked that genUInt already exists in UInt.Gen 😄

  • 😄2