Skip to content

Add toggle function#2554

Closed
bg-wilkesreid wants to merge 2 commits intoramda:masterfrom
bg-wilkesreid:master
Closed

Add toggle function#2554
bg-wilkesreid wants to merge 2 commits intoramda:masterfrom
bg-wilkesreid:master

Conversation

@bg-wilkesreid
Copy link
Copy Markdown

@bg-wilkesreid bg-wilkesreid commented Jun 5, 2018

I've run across a case in which it would be convenient to be able to easily "toggle" a value, which is to return the "opposite" of a value when given two possible values. Basically, if your two possible values are [true, false], this toggle function does the equivalent of value = !value.

Example:

R.toggle("on", ["on", "off"]); //=> "off"

This can be convenient when you would like to toggle a property for an array of objects like so:

R.map(R.evolve({ status: toggle(["active", "inactive"]) }), users)

@CrossEye
Copy link
Copy Markdown
Member

CrossEye commented Jun 5, 2018

Besides the fact that this is failing due to the use of double- rather than single-quotes, I see some serious questions about the API. First of all, Ramda always orders its parameters from least likely to change to most likely to change; this makes it much easier to curry them. So (val, vals) => ... seems backward.

But even if that were fixed to (vals, val) => ..., I still wonder about several things: The implementation suggests that toggle(['on', 'off'], 'foobar') would yield 'off'. Is there any good reason for this? Is this better than 'on'? Moreover, what would be the expected behavior of toggle(['yes', 'no', 'maybe'], 'maybe')? How about toggle(['foo'], 'bar')? And is there a good reason for these choices?

There seem to be two responses. One would suggest that we don't need to limit it to two values, that we might change the name to cycle and allow it to take multiple values. The other would say that there is no need for the array, and that the better API would be (first, second) => current => ... (or likely an uncurried version of that.)

But more importantly than any of these, I question just how important it would be to include this in the library. I've written a function like this once or twice, and it was so easy that I never thought of including it in any utility library. Why do you think it belongs in Ramda?

@bg-wilkesreid
Copy link
Copy Markdown
Author

@CrossEye I completely agree about the order of arguments. As for why it belongs in Ramda, it seems to me that ease of implementation isn't as important as it's potential usefulness and/or convenience, such as with functions like init.

That being said, I thought I'd recommend it be added in case someone else thought it would be a helpful addition.

I've made adjustments to cover those additional conditions you mentioned, change the double quotes to single, and swap the order of the arguments. If it turns out that a curried three-argument API would be better than what I would effectively consider a tuple and a value, then I have no problem with that. I feel like it's unlikely to really need to use currying to provide the two possible values, and that having them contained together in an array makes it more obvious that they are set apart from the value being provided in the case that both the tuple and the value are provided at the same time.

@CrossEye
Copy link
Copy Markdown
Member

CrossEye commented Jun 6, 2018

@bg-wilkesreid:

it seems to me that ease of implementation isn't as important as it's potential usefulness and/or convenience

They are both important questions. For a function that matches Ramda's overall goals of immutability and referential transparency, there are two main questions we ask. First is how widely useful it is. The second is how easy it would be for users to write it themselves if it's not included in Ramda. The more use we think a function is likely to get, the less we worry about the second question. But for functions that do not seem widely applicable, we think fairly deeply about whether the function is something that is tricky enough that an implementation is better in a library. There are other concerns of course. Sometimes we add functions for simple consistency. In a library that has head and tail, if we also add last, we should probably also add init. But such cases are much less common than those for which those first two concerns are paramount.

So my concern is that this is neither a common enough requirement, nor one difficult to implement on one's own. ramda-adjunct and ramda-extension are projects designed to include additional utility functions that don't quite fit in Ramda, and our own Cookbook serves a similar role. Feel free to try to convince us otherwise, that this is more commonly used than I believe. But at the moment, my feeling is that this function would be better placed in one of those locations.

@bg-wilkesreid
Copy link
Copy Markdown
Author

I can appreciate those criteria for the core Ramda library. I'll see about submitting a pull request for one of those adjunct libraries. Thanks!

@tommmyy
Copy link
Copy Markdown
Contributor

tommmyy commented Jun 8, 2018

@bg-wilkesreid @CrossEye We added toggle function with slightly different API, that make sense to us. If you have ideas about how to improve the function please let us know.

@bg-wilkesreid
Copy link
Copy Markdown
Author

@tommmyy Great! I do actually like that API better. It seems more ramda-y.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants