Fix stack overflow in BoundedEnum (Either _ _)

[LiamGoodacre]

The definition of toEnum in the BoundedEnum (Either _ _) instance
eagerly depended on its own instance.

Hid the recursion behind a lambda.
Added a regression test for the instance.

[LiamGoodacre]

Bug spotted by @jacereda

[garyb]

As part of the work for purescript-contrib/purescript-quickcheck-laws#14 I suppose? Thanks both 😄

[jacereda]

Yes, and more is coming.

[garyb]

Ah, merging the other PR has made this one conflict, can you update accordingly please @LiamGoodacre?

[LiamGoodacre]

@paf31 want to drop the cardinality check here too?

[garyb]

I don't think you can, since it's used to determine whether the value is in Left or Right?

[LiamGoodacre]

Yeah just realised :P

[garyb]

In that case, thanks 😉

[LiamGoodacre]

Actually we could drop it from the recursive case?

[LiamGoodacre]

By that I mean, we can get rid of the bit using our own cardinality.

[garyb]

Like this?

    to :: Cardinality a -> Maybe (Either a b)
    to (Cardinality ca)
      | n >= 0 && n < ca = Left <$> toEnum n
      | otherwise = Right <$> toEnum (n - ca)

[LiamGoodacre]

Yeah like that

[garyb]

Sure, works for me!

[jacereda]

Would this be a possibility as well?

to :: Cardinality a -> Maybe (Either a b)
to (Cardinality ca)  = Left <$> toEnum n <|> Right <$> toEnum (n - ca)

Don't know how slow it would be though.

[garyb]

Sure, although yeah I think prefer the other option - we're still making the cardinality check so may as well use that info to avoid the extra toEnum call.

[jacereda]

In the other option I guess you can get rid of Cardinality cab as well, right?

[garyb]

I edited that out immediately after posting it 😄