-
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
Solution for jv-stream-practice #923
base: main
Are you sure you want to change the base?
Conversation
private static final int FROM = 0; | ||
private static final int TO = 1; |
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 believe that these variables are redundant
.filter(person -> | ||
(person.getSex() == Person.Sex.MAN && person.getAge() >= fromAge | ||
&& person.getAge() <= maleToAge) | ||
|| (person.getSex() == Person.Sex.WOMAN | ||
&& person.getAge() >= fromAge | ||
&& person.getAge() <= femaleToAge) |
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.
can be taken out of the stream and create a predicate
Nice job! |
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 comments need to be processed.
private static final int FROM = 0; | ||
private static final int TO = 1; |
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 more descriptive names.
|
||
@Override | ||
public boolean test(Candidate candidate) { | ||
String [] interval = candidate.getPeriodsInUkr().split(SEPARATOR); |
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.
Try to find better name.
int endLivingInUkraine = Integer.parseInt(interval[TO]); | ||
return candidate.getAge() >= MIN_AGE | ||
&& candidate.isAllowedToVote() | ||
&& candidate.getNationality().equals(NATION) |
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.
&& candidate.getNationality().equals(NATION) | |
&& NATION.equals(candidate.getNationality()) |
public int findMinEvenNumber(List<String> numbers) { | ||
return 0; | ||
return numbers.stream() | ||
.flatMap(str -> Stream.of(str.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.
Create constant for comma.
.flatMap(str -> Stream.of(str.split(","))) | ||
.map(Integer::parseInt) | ||
.filter(num -> num % 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.
&& person.getAge() >= fromAge | ||
&& person.getAge() <= toAge) |
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 avoid double getAge
method call.
&& person.getAge() >= fromAge | ||
&& person.getAge() <= femaleToAge) | ||
) | ||
.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(); |
(person.getSex() == Person.Sex.MAN && person.getAge() >= fromAge | ||
&& person.getAge() <= maleToAge) | ||
|| (person.getSex() == Person.Sex.WOMAN | ||
&& person.getAge() >= fromAge | ||
&& person.getAge() <= femaleToAge) |
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 create variables to avoid getAge
and getSex
method calls duplication.
Pay attention to what is a better way to compare Enum values: equals() vs == ?
return Collections.emptyList(); | ||
return peopleList.stream() | ||
.filter(person -> person.getAge() >= femaleAge | ||
&& person.getSex().equals(Person.Sex.WOMAN)) |
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.
&& person.getSex().equals(Person.Sex.WOMAN)) | |
&& Person.Sex.WOMAN.equals(person.getSex())) |
.map(Person::getCats) | ||
.flatMap(Collection::stream) | ||
.map(Cat::getName) | ||
.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.
same
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, some points to consider
.getAsDouble(); | ||
} | ||
|
||
public static boolean isOdd(int number) { |
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.
Should be private
I guess
.filter(person -> { | ||
int age = person.getAge(); | ||
Person.Sex sex = person.getSex(); | ||
return age >= fromAge | ||
&& (Person.Sex.MAN.equals(sex) && age <= maleToAge | ||
|| Person.Sex.WOMAN.equals(sex) && age <= femaleToAge); | ||
}) |
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 we can create separate predicate/method for this long boolean expression, what do you think?
No description provided.