feat: add swap function

[joncloud]

I created a new function R.swap based on the specs listed in #3079.

I based the changes heavily on the implementations of the existing R.move function. I also adjusted the parameter ordering from the main issue to align the currying up with what is suggested later in the thread.

[CrossEye]

I like this idea. There is one requested change.

[CrossEye]

I think this should probably be a shallow clone of the list, perhaps list.slice(0).

In all other cases here we return a new list. Having a no-op case that returns the original value has caused us some issues in the past. I'd like to avoid them.

[joncloud]

Could we leverage the existing result local instead of re-slicing? Also is there a benefit to calling slice(0) vs. slice()?

[CrossEye]

Could we leverage the existing result local instead of re-slicing?

Yes, of course. Anything that achieves that idea is fine.

Also is there a benefit to calling slice(0) vs. slice()?

No, probably not. slice(0) has always been my habit, but not for any good reason.

[joncloud]

Thanks for the feedback! I have rebased, and updated the function to always return a new result.

[CrossEye]

I like this function quite a bit. I'm curious though, why you chose not to implement the remaining idea of

swapArrayByIndex(object, keyA, keyB)
swapArrayByIndex({ a: 1, b: 2}, 'a', 'b')
=> { a:2, b:1 }

[joncloud]

I thought that this was a good boundary for a contribution. I'd be happy to adjust this to include the ability to use objects. Alternately I could also contribute to implement objects for both R.move and R.swap. Let me know!

[CrossEye]

My take is that this is reasonable for swap, and if we're going to add the function, it makes sense to do so in the same PR. But it's much less of a clear API for move.

I think we need to decide what sort of type we are thinking of this swap working on. Essentially it's an indexed structure of some sort, something broad enough to encompass both Arrays and Objects. But I don't have a good name for it.

[joncloud]

I can update my request to contain the ability to swap an array or an object, and we can defer the move implementation.

As for determining the type: is there a concept for a union?

[CrossEye]

I have no problem with an ad hoc union in the type signature. I'm just wondering if there is an abstract type we should have in the background, in our thoughts here. For instance, should this work?:

swap (0, 2) ('foo') //=> 'oof'

or this?:

swap (obj1, obj2) (new Map ([[obj1, 1] [obj2, 2]])) //=> new Map ([[obj1, 2], [obj2, 1]])

Is there a general abstraction here that various types might match and expect this to work.

But I don't want this to slow down the PR. If we do find one, we can expand the idea later. I'm mostly just musing here.

[Bradcomp]

We already have an Index type we use for functions like path. It's an ad-hoc union but at least it's an existing ad-hoc union. I think it makes sense for this function to work on indexable things (currently Array and Object), and would make it more useful. An issue with the type signature that isn't present in path is that for this function, both of the two indexes should be the same type.

But do they? Technically an Array can have string indexed members, and an Object will coerce obj[1] to obj['1']. Do we want to care about that? After typing this out my thought is that it's fine to take two arbitrary Index values, but I also have a fairly high tolerance for the looseness of JS in the context of my typical Ramda use case.

[CrossEye]

I'm not sure enough of my HM annotation syntax to get this right, but I think there's something like this possible:

swap :: Idx a, Indexed a b => a -> a -> b -> b
Idx = String | Int
Indexed a b = ???

and this would enforce that the indices are the same type. Since our types are aspirational rather than enforced, I would say that yes, we would say that the indices should be the same type.

[joncloud]

I made some more adjustments based on some of the feedback above to account for swapping more types. I also made some minor adjustments to the documentation, but I am sure that at least @sig will need some attention.

Let me know what you think, and I can make more changes.

[CrossEye]

This looks good to me.

And I think it's worth doing.

Any thoughts from @ramda/core?

[davidchambers]

I have a strong negative reaction to this addition. Parametric polymorphism is unquestionably wonderful, but ad hoc polymorphism is a source of complexity. Disciples of Rich Hickey appreciate that strands hanging apart are simpler than strands intertwined (although amusingly many core Clojure functions are “complected” by ad hoc polymorphism).

My second objection is that the proposed function uses indices. Indices are a source of complexity and bugs.

I'm sorry to be negative, @joncloud. The pull request is of high quality; I just question the objective.

[joncloud]

No worries @davidchambers. I appreciate the feedback both on the concept, as well as the pull request.

[hitmands]

The main takeaway from me is around the purpose of this function and his place in the world along with R.move,
as a user I would probably esitate on which I should pick... Probably I'd suggest to add some example around this.

Implementation looks great to me.

[CrossEye]

@davidchambers: Would your objections to ad hoc polymorphism be satisfied if we did formally name the shared type and noted some laws that it must follow? I'd given some thought to that, but hadn't yet reached any conclusions. Clearly it has something to do with keyed data structures, but I haven't worked it out yet.

As to indices, that is built into a number of Ramda functions. JS doesn't treat arrays as only head-tail pairs -- actually that's fairly unusual, although I like it -- but also as directly addressable vectors. I personally don't have any trouble with the fact that this function, for arrays, deals with indices. To me that's no different than the fact that for objects, it deals with String keys.

[davidchambers]

Would your objections to ad hoc polymorphism be satisfied if we did formally name the shared type and noted some laws that it must follow?

Yes. At that point the function would have one type signature rather than several.

As to indices, that is built into a number of Ramda functions.

True. Sanctuary has no such functions. ;)