Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lens-transform should be consistent with lens-set #72

Closed
jackfirth opened this issue Jul 7, 2015 · 6 comments · Fixed by #77
Closed

lens-transform should be consistent with lens-set #72

jackfirth opened this issue Jul 7, 2015 · 6 comments · Fixed by #77
Labels
Milestone

Comments

@jackfirth
Copy link
Owner

lens-set takes the target before the new value, while lens-transformer takes the target after the transformer. They should both take the target before.

@jackfirth jackfirth added this to the 1.0 milestone Jul 7, 2015
@AlexKnauth
Copy link
Collaborator

Or, does the argument order of lens-set* and lens-transform* make more sense? (target first, then lens, then new-value/transformer-function)

It breaks the convention of having the lens as the first argument, but allows the lens and the new-value/transformer-function to be next to each other, and is more consistent with hash-set, hash-update, and other similar functions.

@jackfirth
Copy link
Owner Author

I'm not sure how to fold plurality into argument conventions, because there's several plural cases:

  • Multiple lenses, one target, one view (lens-view*)
  • Multiple lenses, one target, multiple views (lens-transform*, lens-set*)
  • One lens, multiple targets, one view (possibly something like lens-set-all, lens-transform-all)
  • One lens, multiple targets, multiple views (possibly something like lens-set-each, lens-transform-each)
  • Multiple lenses, multiple targets, one view (possibly something like lens-set-all for multiple lenses)

And probably more I haven't thought of. The singular lens-view couldn't be both consistent with a function that takes multiple lenses and a function that takes multiple views if variable-arity functions are desired and the rest arguments are uniform.

@AlexKnauth
Copy link
Collaborator

Well, right now, the argument orders for the single-lens cases of lens-view*, lens-set* and lens-transform* are consistent with each other and consistent with functions like hash-ref, hash-set, and hash-update.

If you thought it would make more sense, you could change all of them to follow that convention, but that breaks the convention of having the lens as the first argument, and also turns it into a postfix-ish feel.

I'm not sure what you mean by lens-view being consistent with a function that takes multiple views though?

@jackfirth
Copy link
Owner Author

I meant lens-set, my apologies.

@AlexKnauth
Copy link
Collaborator

Maybe worth noting: For the plural of lens-set, I debated between two possibilities:

  • (lens-set* target lens ... new-val), equivalent to (lens-set (lens-thrush lens ...) target new-val)
  • (lens-set* target lens new-val ... ...), equivalent to (for/fold ([target target]) ([lens/val (in-slice 2 ....)]) (lens-set lens target val)), filling in the gaps.

I chose the second one because I thought it would be more useful, especially since it's easy to throw a lens-thrush in there for the first one, and also because the second would be consistent with hash-set*.

@jackfirth
Copy link
Owner Author

Let's move general discussion of argument order to #73

AlexKnauth added a commit to AlexKnauth/lens that referenced this issue Jul 8, 2015
@jackfirth jackfirth removed the ready label Jul 8, 2015
jackfirth added a commit that referenced this issue Jul 8, 2015
change argument order of lens-transfrom, closes #72
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants