Immutable EnumSet #109
Merged
Immutable EnumSet #109
Conversation
This file contains hidden or 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
prolic
approved these changes
Feb 7, 2019
There was a problem hiding this comment.
Looks good to me.
Maybe it's also not too unreasonable to have EnumSet/Map and EnumSet/Map-Immutable classes, similar to DateTime and DateTimeImmutable, so users have the choice to use what they prefer. I'd say immutable is always to prefer because it doesn't come with side-effects which can lead to tricky bugs, but no one listens to me anyway :)
|
I'm not entirely sure about this. It would make sense only if both classes implement the same Interface. This means the interface would not make sure to handle immutable and mutable objects - in with DateTime and DateTimeImmutable. Let's fist see what are the performance differences |
marc-mabe
force-pushed
the
immutable-set
branch
from
56a84d2 to
d2c50e0
Compare
|
Here is the full benchmark: For me it tells me I should check where all the performance degrations comes from.
|
marc-mabe
force-pushed
the
immutable-set
branch
from
d2c50e0 to
4c41ffe
Compare
marc-mabe
force-pushed
the
immutable-set
branch
7 times, most recently
from
ad8f316 to
209f028
Compare
|
@prolic It's ready now :) I decided to have both mutable and immutable interfaces but with consistent naming:
|
prolic
requested changes
Mar 5, 2019
marc-mabe
force-pushed
the
immutable-set
branch
from
209f028 to
85bbc20
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Based on #108
EnumSetas immutableattach(): void->withEnumerator(): selfdetach(): void->withoutEnumerator(): selfsetBit(): void->withBit(): selfsetBinaryBitset[Le|Be](): void->withBinaryBitset[Le|Be](): selfPS 1: I still want to make some benchmarks for comparison
PS 2: This make sense only if we make
EnumMapimmutable too@prolic ping