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
Added a partition
method to all containers
#1916
Merged
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
4fd5620
Added basic definition of parition method
johnw42 5b177d3
completed types for parition()
johnw42 04a331c
all tests pass for partition()
johnw42 337d7e5
revised README.md
johnw42 a0eae58
added typescript docs
johnw42 d64d016
fixed lint errors; removed yarn.lock
johnw42 a668615
Reverted accidental changes in README.md
johnw42 892fd29
reverted accidental change to next-env.d.ts
johnw42 File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,102 @@ | ||
import { | ||
Collection, | ||
isAssociative, | ||
isIndexed, | ||
isKeyed, | ||
isList, | ||
isMap, | ||
isSeq, | ||
isSet, | ||
List, | ||
Map as IMap, | ||
Seq, | ||
Set as ISet, | ||
} from 'immutable'; | ||
|
||
describe('partition', () => { | ||
let isOdd: jest.Mock<unknown, [x: number]>; | ||
|
||
beforeEach(() => { | ||
isOdd = jest.fn(x => x % 2); | ||
}); | ||
|
||
it('partitions keyed sequence', () => { | ||
const parts = Seq({ a: 1, b: 2, c: 3, d: 4 }).partition(isOdd); | ||
expect(isKeyed(parts[0])).toBe(true); | ||
expect(isSeq(parts[0])).toBe(true); | ||
expect(parts.map(part => part.toJS())).toEqual([ | ||
{ b: 2, d: 4 }, | ||
{ a: 1, c: 3 }, | ||
]); | ||
expect(isOdd.mock.calls.length).toBe(4); | ||
|
||
// Each group should be a keyed sequence, not an indexed sequence | ||
const trueGroup = parts[1]; | ||
expect(trueGroup && trueGroup.toArray()).toEqual([ | ||
['a', 1], | ||
['c', 3], | ||
]); | ||
}); | ||
|
||
it('partitions indexed sequence', () => { | ||
const parts = Seq([1, 2, 3, 4, 5, 6]).partition(isOdd); | ||
expect(isIndexed(parts[0])).toBe(true); | ||
expect(isSeq(parts[0])).toBe(true); | ||
expect(parts.map(part => part.toJS())).toEqual([ | ||
[2, 4, 6], | ||
[1, 3, 5], | ||
]); | ||
expect(isOdd.mock.calls.length).toBe(6); | ||
}); | ||
|
||
it('partitions set sequence', () => { | ||
const parts = Seq.Set([1, 2, 3, 4, 5, 6]).partition(isOdd); | ||
expect(isAssociative(parts[0])).toBe(false); | ||
expect(isSeq(parts[0])).toBe(true); | ||
expect(parts.map(part => part.toJS())).toEqual([ | ||
[2, 4, 6], | ||
[1, 3, 5], | ||
]); | ||
expect(isOdd.mock.calls.length).toBe(6); | ||
}); | ||
|
||
it('partitions keyed collection', () => { | ||
const parts = IMap({ a: 1, b: 2, c: 3, d: 4 }).partition(isOdd); | ||
expect(isMap(parts[0])).toBe(true); | ||
expect(isSeq(parts[0])).toBe(false); | ||
expect(parts.map(part => part.toJS())).toEqual([ | ||
{ b: 2, d: 4 }, | ||
{ a: 1, c: 3 }, | ||
]); | ||
expect(isOdd.mock.calls.length).toBe(4); | ||
|
||
// Each group should be a keyed collection, not an indexed collection | ||
const trueGroup = parts[1]; | ||
expect(trueGroup && trueGroup.toArray()).toEqual([ | ||
['a', 1], | ||
['c', 3], | ||
]); | ||
}); | ||
|
||
it('partitions indexed collection', () => { | ||
const parts = List([1, 2, 3, 4, 5, 6]).partition(isOdd); | ||
expect(isList(parts[0])).toBe(true); | ||
expect(isSeq(parts[0])).toBe(false); | ||
expect(parts.map(part => part.toJS())).toEqual([ | ||
[2, 4, 6], | ||
[1, 3, 5], | ||
]); | ||
expect(isOdd.mock.calls.length).toBe(6); | ||
}); | ||
|
||
it('partitions set collection', () => { | ||
const parts = ISet([1, 2, 3, 4, 5, 6]).partition(isOdd); | ||
expect(isSet(parts[0])).toBe(true); | ||
expect(isSeq(parts[0])).toBe(false); | ||
expect(parts.map(part => part.toJS().sort())).toEqual([ | ||
[2, 4, 6], | ||
[1, 3, 5], | ||
]); | ||
expect(isOdd.mock.calls.length).toBe(6); | ||
}); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
andfalse
; 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well I like the array better than the object, because object with
true
andfalse
keys, even if they work, might be misinterpreted.Moreover, you can't do stuff like that:
In that case, your implementation seems a good approach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, good point. I'm happy to leave it as-is.