search

emilypi / smash

Smash products, Wedge products, and other Pointed stuff
34 stars 11 forks source link

Indexed traversals in smash-optics use Maybe Bool as index but Nothing is never used #11

Closed arybczak closed 4 years ago

arybczak commented 4 years ago

As far as I can see they should be indexed by Bool.

emilypi commented 4 years ago

Indexing by ternary value seems to me the better model for this, but I do agree that I'm abusing the Applicative instance for Maybe to get around the use of the Nothing index. For example, 

Non -> pure Non

could be

Non -> f Nothing Non

I'm unsold as to which is better, and I'm open to ideas. Looking at the instances in Optics.Each.Core, there doesn't seem to be a hard and fast rule considering we allow for Int-indexed containers (what to do with n < 0?), etc. If someone changed this to Bool, I would not protest too hard.

arybczak commented 4 years ago

I'm abusing the Applicative instance for Maybe to get around the use of the Nothing index.

I don't know what you mean by that :thinking:

Non -> Non <$ f Nothing

That won't work, because f expects 2 arguments and there is nothing you can pass as the second one.

I agree that there are no hard rules, though intuitively I'd say one doesn't expect a traversal indexed by Int to cover the whole range, but for small index types they might.

emilypi commented 4 years ago

@arybczak Ah you're right. I am getting my types mixed up. To use the Nothing case means we have to have more information than we do.

Bool it is.

emilypi commented 4 years ago

Closing because this is now merged. Thanks @arybczak !