-
Notifications
You must be signed in to change notification settings - Fork 39
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
Introduce CollectionIterator
Refaster rule
#1347
Conversation
Looks good. No mutations were possible for these changes. |
By re-writing `Stream#iterator` statements.
beb74e6
to
012b5e4
Compare
Looks good. No mutations were possible for these changes. |
Quality Gate passedIssues Measures |
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.
Added a commit and updated the suggested commit message. Nice suggestion @mohamedsamehsalah!
* Don't call {@link ImmutableCollection#asList()} if {@link ImmutableCollection#iterator()} is | ||
* called on the result; call it directly. | ||
*/ | ||
/** Prefer {@link ImmutableCollection#iterator()} over more contrived alternatives. */ |
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.
We can also call out that these alternatives are likely less performant.
static final class ImmutableCollectionIterator<T> { | ||
@BeforeTemplate | ||
Iterator<T> before(ImmutableCollection<T> collection) { | ||
return collection.asList().iterator(); | ||
return Refaster.anyOf(collection.stream().iterator(), collection.asList().iterator()); |
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.
I think it's due to copy-pasta from the rule above, but we can actually generalize the first expression to any Collection
.
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.
Makes sense. I missed the generalization part 😅
Thanks @Stephan202 ❤️
return ImmutableSet.of(1).asList().iterator(); | ||
ImmutableSet<Iterator<Integer>> testImmutableCollectionIterator() { | ||
return ImmutableSet.of( | ||
ImmutableSet.of(1).stream().iterator(), ImmutableSet.of(1).asList().iterator()); |
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.
ImmutableSet.of(1).stream().iterator(), ImmutableSet.of(1).asList().iterator()); | |
ImmutableSet.of(1).stream().iterator(), ImmutableSet.of(2).asList().iterator()); |
It was actually your suggestion 🚀 Thanks for review. 😃 |
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.
Nice 🚀 !
ImmutableCollectionIterator
refaster ruleCollectionIterator
Refaster rule
Suggested commit message