New: swap #42
New: swap #42
Conversation
|
Nice. Just do two things for me:
|
|
@Harris-Miller thanks for the quick response! On it. |
|
@Harris-Miller can you take another look? Note: I had some lint errors. I fixed those and also updated |
| @@ -0,0 +1,58 @@ | |||
| import { Placeholder } from './util/tools'; | |||
|
|
|||
| // swap(indexA) | |||
There was a problem hiding this comment.
I really like the comments! Makes all these overloads much easier to follow 👍🏻
| indexB: number | ||
| ): { | ||
| // swap(__, indexB)(indexA)(list) | ||
| (indexA: number): <T>(list: readonly T[]) => T[]; |
There was a problem hiding this comment.
I'm curious about the readonly keyword here. Is this just preventing the definition of swap from mutating the list argument? I guess the implementation of swap is already written in the other repo and these definitions do not restrict that implementation.
Does readonly do anything else that restricts the consumer or type inference in any way? It probably can't hurt to have these here, but just wanted to see if I was missing something else.
There was a problem hiding this comment.
You're spot-on, but I wouldn't describe it as "only" doing what it does because I think it's a pretty powerful TS feature.
If you have a readonly array and a function that takes an array, you have no guarantee that the function won't modify the array. Thus, TS won't let you pass your array into the function. If you make the argument readonly, then you're telling the caller that you won't modify it. Plus, a non-readonly array basically extends a readonly array, so it's also a valid argument to the function. So slapping a readonly on your parameter means you can accept any argument, readonly or not.
Every function in Ramda is a pure function so there should be readonlys throughout, which just means that you can call them with whatever you want. I've run into really annoying libraries that don't include the readonly even though they actually are. If you have a dropdown component, then define const DROPDOWN_OPTIONS = [...] as const, if the dropdown typing doesn't have the options as readonly you won't be able to pass that in.
There was a problem hiding this comment.
ahh, that makes sense. It does help the consumer in that this function now accepts readonly arrays. Thanks for the explanation!
Ramda 0.29.0 introduced a simple swap function. Here's the typing for it. Full disclosure: I copy/pasted the code from
adjust.d.tssince the function signatures are similar.