Skip to content
This repository has been archived by the owner on Oct 22, 2024. It is now read-only.

Refactor UnionSet deduplicating iterable #117

Merged
merged 1 commit into from
Feb 4, 2020
Merged

Conversation

natebosch
Copy link
Contributor

Set.add returns a boolean so it is unnecessary to use both
Set.contains and then Set.add. Once the .where call is shortened
to a tearoff the deduplication can be inlined with _iterable.

`Set.add` returns a boolean so it is unnecessary to use both
`Set.contains` and then `Set.add`. Once the `.where` call is shortened
to a tearoff the deduplication can be inlined with `_iterable`.
@kevmoo
Copy link
Contributor

kevmoo commented Feb 2, 2020

@natebosch – how'd you find THIS?!?!

@natebosch
Copy link
Contributor Author

Saw some usage, wasn't familiar with the class and I was curious how much it was accomplishing. Jumped in to look at the implementation and the contains then add pattern stood out to me.

/// values.
Iterable<E> get _iterable {
var allElements = _sets.expand((set) => set);
return _disjoint ? allElements : allElements.where(<E>{}.add);
Copy link
Contributor

@jakemac53 jakemac53 Feb 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While we are in here we might as well also make this a HashSet nevermind this actually seems to make things a bit worse 🤷‍♂️

@natebosch natebosch merged commit 7be42e0 into master Feb 4, 2020
@natebosch natebosch deleted the dedup-iterable branch February 4, 2020 19:15
mosuem pushed a commit to dart-lang/core that referenced this pull request Oct 18, 2024
`Set.add` returns a boolean so it is unnecessary to use both
`Set.contains` and then `Set.add`. Once the `.where` call is shortened
to a tearoff the deduplication can be inlined with `_iterable`.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants