Added a partition method to all containers
#1916
Conversation
johnw42
changed the title
partition method to all containerspartition method to all containers
|
Il does seems nice. I will check that. |
|
Thanks! I think I should draw your attention to the change in |
| @@ -389,6 +389,18 @@ export function groupByFactory(collection, grouper, context) { | |||
| return groups.map(arr => reify(collection, coerce(arr))).asImmutable(); | |||
| } | |||
|
|
|||
| export function partitionFactory(collection, predicate, context) { | |||
| const isKeyedIter = isKeyed(collection); | |||
| const groups = [[], []]; | |||
There was a problem hiding this comment.
What do you think of using a immutable Set for the main container ?
It seems coherent with the groupBy function that returns an immutable instance ?
There was a problem hiding this comment.
A Set has the wrong semantics because the result is ordered. As for using something else like a List, there are a couple of reasons why I didn't do it that way. One is that I expect the array to be immediately destructured in typical use, so creating an instance of an immutable type would add extra overhead that rarely has any benefit. The other is that it would break the TypeScript type of the result. In TypeScript the result is guaranteed to have exactly two values, and they even have different types when the predicate is a type-testing function.
I think the only other type that really makes sense is a native object type with fields named something like true and false; it would make the meaning the fields more clear, and it has no more overhead than an array. (I checked, and "true" and "false" work fine as field names.) Now that I think of it, I think returning an object is probably the the best, most idiomatic solution all around.
There was a problem hiding this comment.
Well I like the array better than the object, because object with true and false keys, even if they work, might be misinterpreted.
Moreover, you can't do stuff like that:
const { false, true } = partitionned;In that case, your implementation seems a good approach.
There was a problem hiding this comment.
Yeah, good point. I'm happy to leave it as-is.
|
@johnw42 it has been release in |
This PR adds the
partitionmethod common in other libraries and languages to all immutable containers. It contains the implementation as well as updated documentation, TypeScript types, and unit tests. I have not added Flow support because I'm not familiar enough with it. See changes toREADMD.mdfor more details.