Add Lens implementation for tuples #1

Closed
julien-truffaut opened this Issue Jan 19, 2014 · 11 comments

Comments

Projects
None yet
3 participants
@julien-truffaut
Owner

julien-truffaut commented Jan 19, 2014

e.g. _1.set((1, 3), 2) ==> (2,3)
_2.modify((2, "London"), reverse) ==> (2, "nodnoL")

Bonus task: think about easier syntax

@rossh

This comment has been minimized.

Show comment Hide comment
@rossh

rossh Jan 19, 2014

Contributor

@julien-truffaut I can take this. You can assign it to me if you want (rossh).

Contributor

rossh commented Jan 19, 2014

@julien-truffaut I can take this. You can assign it to me if you want (rossh).

@ghost ghost assigned rossh Jan 19, 2014

rossh pushed a commit to rossh/Monocle that referenced this issue Mar 8, 2014

rossh pushed a commit to rossh/Monocle that referenced this issue Mar 8, 2014

@rossh

This comment has been minimized.

Show comment Hide comment
@rossh

rossh Mar 8, 2014

Contributor

@julien-truffaut This is done. Please review and merge if happy.

Contributor

rossh commented Mar 8, 2014

@julien-truffaut This is done. Please review and merge if happy.

@rossh rossh closed this Mar 8, 2014

@NightRa

This comment has been minimized.

Show comment Hide comment
@NightRa

NightRa Mar 8, 2014

Collaborator

The lens name is too verbose, and there isn't a lens for the first arg in a tuple.

Collaborator

NightRa commented Mar 8, 2014

The lens name is too verbose, and there isn't a lens for the first arg in a tuple.

@NightRa

This comment has been minimized.

Show comment Hide comment
@NightRa

NightRa Mar 8, 2014

Collaborator

Is there a plan for supporting lenses for all tuples or pairs only?

Collaborator

NightRa commented Mar 8, 2014

Is there a plan for supporting lenses for all tuples or pairs only?

@julien-truffaut

This comment has been minimized.

Show comment Hide comment
@julien-truffaut

julien-truffaut Mar 8, 2014

Owner

We want to make it for all tuples and also use Lens and not SimpleLens.
This is the first version, I will let this issue open to track progress.

Owner

julien-truffaut commented Mar 8, 2014

We want to make it for all tuples and also use Lens and not SimpleLens.
This is the first version, I will let this issue open to track progress.

@rossh

This comment has been minimized.

Show comment Hide comment
@rossh

rossh Mar 8, 2014

Contributor

@NightRa How about _1 and _2?

Contributor

rossh commented Mar 8, 2014

@NightRa How about _1 and _2?

@NightRa

This comment has been minimized.

Show comment Hide comment
@NightRa

NightRa Mar 8, 2014

Collaborator

@rossh Not sure it's a valid name.
For tuple2, very nice names could be: fst and snd, just like the accessors in haskell.
For higher tuples, if _1,_2,_3,... is possible, it could be used, but it's not the nicest,
And we would need to somehow overload them for every tuple type, which I don't know how to do.

Collaborator

NightRa commented Mar 8, 2014

@rossh Not sure it's a valid name.
For tuple2, very nice names could be: fst and snd, just like the accessors in haskell.
For higher tuples, if _1,_2,_3,... is possible, it could be used, but it's not the nicest,
And we would need to somehow overload them for every tuple type, which I don't know how to do.

@julien-truffaut

This comment has been minimized.

Show comment Hide comment
@julien-truffaut

julien-truffaut Mar 8, 2014

Owner

We might be able to use a type class like First[T] that will define how to get and set first element of a T and then implement it for each tuple.
Otherwise we could try to convert tuples to HList and write lens on HList if that's easier

Owner

julien-truffaut commented Mar 8, 2014

We might be able to use a type class like First[T] that will define how to get and set first element of a T and then implement it for each tuple.
Otherwise we could try to convert tuples to HList and write lens on HList if that's easier

@NightRa

This comment has been minimized.

Show comment Hide comment
@NightRa

NightRa Mar 8, 2014

Collaborator

@julien-truffaut I think converting to an HList makes more sense than making 22 type classes.
But when we work with HLists, there is a performance impact.

Collaborator

NightRa commented Mar 8, 2014

@julien-truffaut I think converting to an HList makes more sense than making 22 type classes.
But when we work with HLists, there is a performance impact.

@julien-truffaut

This comment has been minimized.

Show comment Hide comment
@julien-truffaut

julien-truffaut Mar 8, 2014

Owner

@NightRa you"re right. Maybe we can go for an hybrid solution: type classes up to reasonable size (e.g. 5) and then HList conversion for bigger tuples.

Owner

julien-truffaut commented Mar 8, 2014

@NightRa you"re right. Maybe we can go for an hybrid solution: type classes up to reasonable size (e.g. 5) and then HList conversion for bigger tuples.

@rossh

This comment has been minimized.

Show comment Hide comment
@rossh

rossh Mar 9, 2014

Contributor

@julien-truffaut @NightRa I'm a bit lost but I'm new to Scala so I'll take you guys word for it! Once I get more experience, I'll probably be able to give better input.

Contributor

rossh commented Mar 9, 2014

@julien-truffaut @NightRa I'm a bit lost but I'm new to Scala so I'll take you guys word for it! Once I get more experience, I'll probably be able to give better input.

julien-truffaut pushed a commit that referenced this issue Nov 15, 2015

Merge pull request #1 from LiamGoodacre/test-cofree
add Eq and Arbitrary instance for Cofree and fix issue in toStream
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment