Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

TAJO-2058: foreach loop can be collapsed with stream api #944

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

dkhwangbo
Copy link
Contributor

I replace many foreach loop with stream api.

for (PartitionSpecifier specifier : specifiers) {
hash.specifiers.add(specifier);
}
hash.specifiers.addAll(specifiers.stream().collect(Collectors.toList()));
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually in a case like this, I think it's sufficient and more clear.

hash.specifiers.addAll(specifiers);

or more simply,

hash.specifiers = new ArrayList<>(specifiers);

Anyway, I think stream doesn't matter.
Because it is simple shallow copy, there are many ways.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@eminency Thanks for your comment. I agree with your opinion.
I'll rewrite it overall correlated with a simple shallow copy.

@eminency
Copy link
Contributor

eminency commented Mar 9, 2016

Time to merge.

@dkhwangbo
Copy link
Contributor Author

@eminency Thanks for your comment. Unfortunately, I face with some unsuspected error while travis test. I'm in investigation about it's cause.

for (PartitionSpecifier specifier : specifiers) {
hash.specifiers.add(specifier);
}
hash.specifiers = specifiers.stream().collect(Collectors.toList());
Copy link
Contributor

Choose a reason for hiding this comment

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

It is sufficient with:

hash.specifiers = new ArrayList<>(specifiers);

@eminency
Copy link
Contributor

Even though they may not exact, I left some comments.
Please check them out.

@dkhwangbo
Copy link
Contributor Author

@eminency Hi! Thanks for your comment. I apply your comment.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants