purescript-contrib / purescript-profunctor-lenses

Pure profunctor lenses
MIT License
144 stars 51 forks source link

Adds filterMapped #143

Closed mikesol closed 2 years ago

mikesol commented 2 years ago

Description of the change


This adds support for a case I've encountered a few times where one is working with a lookup table. In those cases, filtered doesn't cut it because a boolean predicate won't get us the new value from the table. For this, it's better to have something that resembles filterMap.

The only unconventional thing here is that the lens modifies the value on its way down via the mapping, but it sort of makes sense if you think of the value's presence in the lookup table as a property of the value itself.

Checklist:

MonoidMusician commented 2 years ago

I guess I have two concerns:

Personally I would suggest something like traversed <<< lens identity fromMaybe :: forall t a. Traversable t => Traversal (t a) (t a) a (Maybe a). This should allow you to discard modifying the value when you return Nothing. For example, your test example becomes the following:

over (traversed <<< lens identity fromMaybe) (flip lookup myMap >>> map (add 42)) [1,2,3,4]

That said, even that isn't providing much value over my favorite idiom: apply fromMaybe :: (a -> Maybe a) -> (a -> a). So it could just be

over traversed (fromMaybe <*> flip lookup myMap >>> map (add 42)) [1,2,3,4]

But maybe it isn't your favorite idiom :joy:

mikesol commented 2 years ago

@MonoidMusician the criticism is good and could be a reason to ultimately not merge. Related to the second point, I think we'd have to agree on what a property is with respect to a parent entity. If I take me, my name, and my name in Chinese, most folks would be ok with a Lens from me to my name. But me to my name in Chinese requires a lookup table, and it's possible that the name doesn't exist, so it's close to lucky. One way to think of it would be "is 'my name in Chinese' as admissable as a zoom-able property as 'my name'?" If so, that could be an argument for the PR.

Regarding the first point, I have to digest more, but I can't yet spot where this breaks the lens laws. Do you have an example in mind?

MonoidMusician commented 2 years ago

lucky Just and lucky (const Nothing) are fine, but lucky (flip lookup myMap) does not have the property that over optic identity = identity (or set l (view l s) s = s from https://hackage.haskell.org/package/lens-5.1/docs/Control-Lens-Lens.html#t:Lens). And it's precisely the enmeshment of external data in the lens itself that means this law fails.

Said another way, if you really do need extra data, to work within the lens philosophy you should integrate that extra data into the datatype already, or reference it while doing operations, as opposed to zipping it in while making the lens.

mikesol commented 2 years ago

Got it, that's quite convincing! I'll close the PR and rewrite my lenses using the strategy you mention here of including the data (the lookup table) in the lens itself.