-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add implementation for the Stream API Practice task #910
base: main
Are you sure you want to change the base?
Conversation
… Predicate<Candidate> interface.
Clean up code Pass tests.
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.
Good job! But a few points need to be processed.
.flatMap(s -> Arrays.stream(s.split(","))) | ||
.map(Integer::parseInt) | ||
.toList(); | ||
return allNumbers |
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.
Make it one stream.
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.
fixed
return allNumbers | ||
.stream() | ||
.filter(n -> n % 2 == 0) | ||
.min(Integer::compareTo) |
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 you can call min()
without arguments.
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.
fixed
.range(0, numbers.size()) | ||
.filter(i -> i % 2 != 0) | ||
.forEach(i -> numbers.set(i, numbers.get(i) - 1)); | ||
return numbers |
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.
Make one stream for all logic here.
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.
fixed
.forEach(i -> numbers.set(i, numbers.get(i) - 1)); | ||
return numbers | ||
.stream() | ||
.filter(n -> n % 2 != 0) |
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.
Think abut avoiding n -> n % 2 != 0
code duplication.
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.
fixed
List<Integer> allNumbers = numbers | ||
.stream() | ||
.flatMap(s -> Arrays.stream(s.split(","))) |
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.
List<Integer> allNumbers = numbers | |
.stream() | |
.flatMap(s -> Arrays.stream(s.split(","))) | |
List<Integer> allNumbers = numbers.stream() | |
.flatMap(s -> Arrays.stream(s.split(","))) |
It would be better not place stream() in new line.
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.
fixed
return Collections.emptyList(); | ||
return peopleList | ||
.stream() | ||
.filter(person -> person.getSex().equals(Person.Sex.MAN) |
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.
.filter(person -> person.getSex().equals(Person.Sex.MAN) | |
.filter(person -> Person.Sex.MAN.equals(person.getSex()) |
Let's avoid NPE.
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.
fixed
.filter(person -> person.getSex().equals(Person.Sex.MAN) | ||
&& person.getAge() >= fromAge | ||
&& person.getAge() <= toAge) | ||
.collect(Collectors.toList()); |
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.
.collect(Collectors.toList()); | |
.toList(); |
Be sure to make it everywhere.
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.
fixed
.filter(person -> person.getAge() >= fromAge | ||
&& (person.getSex().equals(Person.Sex.WOMAN) | ||
&& person.getAge() <= femaleToAge | ||
|| person.getSex().equals(Person.Sex.MAN) | ||
&& person.getAge() <= maleToAge)) |
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.
getAge()
3 times.
getAge()
2 times.
Lets make variables for it.
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.
fixed
@Override | ||
public boolean test(Candidate candidate) { | ||
String[] periodInUkr = candidate.getPeriodsInUkr().split("-"); | ||
int yearsInUkr = Integer.parseInt(periodInUkr[1]) - Integer.parseInt(periodInUkr[0]); |
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.
Let's make constants for indexes here.
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.
fixed
return 0; | ||
List<Integer> allNumbers = numbers | ||
.stream() | ||
.flatMap(s -> Arrays.stream(s.split(","))) |
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.
Let's make constant for regex.
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.
fixed
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.
Good job.
A few minor comments need to be processed.
@@ -14,16 +20,31 @@ public class StreamPractice { | |||
* "Can't get min value from list: < Here is our input 'numbers' >" | |||
*/ | |||
public int findMinEvenNumber(List<String> numbers) { | |||
return 0; | |||
return numbers.stream() | |||
.flatMap(s -> Arrays.stream(s.split(STRING_SPLITTER))) |
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.
Let's dont use one-letter names.
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 guess in this kind of situations it's like i in for, fixed.
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.
Good job!
No description provided.