Hw jv stream #1204
Hw jv stream #1204
Conversation
There was a problem hiding this comment.
Great job on implementing the task requirements! 🎉 Your code meets the core functionality, and you've effectively used Java Streams and custom predicates. There are a few minor improvements suggested, like formatting the exception message, using constants, and enhancing readability with stream method chaining. These are not critical but will help in maintaining and scaling your code in the future. Keep up the good work and happy coding! 😊
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
|
|
||
| @Override | ||
| public boolean test(Candidate candidate) { | ||
| if (candidate.getAge() < 35) { |
There was a problem hiding this comment.
create constants for 35 and other magic numbers |
also for comma and numbers
| String[] range = period.trim().split("-"); | ||
| int startYear = Integer.parseInt(range[0]); | ||
| int endYear = Integer.parseInt(range[1]); | ||
| return endYear - startYear + 1; // Учитываем включение последнего года |
| @Override | ||
| public boolean test(Candidate candidate) { | ||
| if (candidate.getAge() < 35) { | ||
| return false; |
There was a problem hiding this comment.
You can refactor your code like
return candidate.getAge() >= MIN_AGE
&& candidate.isAllowedToVote()
&& candidate.getNationality().equals(REQUIRED_NATIONALITY)
&& checkTimeLivingInCountry(candidate);
| @@ -35,7 +49,10 @@ public Double getOddNumsAverage(List<Integer> numbers) { | |||
| * Example: select men who can be recruited to army (from 18 to 27 years old inclusively). | |||
| */ | |||
| public List<Person> selectMenByAge(List<Person> peopleList, int fromAge, int toAge) { | |||
| return Collections.emptyList(); | |||
| return peopleList.stream() | |||
| .filter(p -> p.getSex().equals(Person.Sex.MAN) | |||
| return peopleList.stream() | ||
| .filter(p -> p.getSex().equals(Person.Sex.MAN) | ||
| && (p.getAge() >= fromAge && p.getAge() <= toAge)) | ||
| .collect(Collectors.toList()); |
There was a problem hiding this comment.
| .collect(Collectors.toList()); | |
| .toList(); |
| @@ -50,7 +67,12 @@ public List<Person> selectMenByAge(List<Person> peopleList, int fromAge, int toA | |||
| */ | |||
| public List<Person> getWorkablePeople(int fromAge, int femaleToAge, | |||
| int maleToAge, List<Person> peopleList) { | |||
| return Collections.emptyList(); | |||
| return peopleList.stream() | |||
| .filter(p -> (p.getSex().equals(Person.Sex.MAN) | |||
| @@ -59,7 +81,11 @@ public List<Person> getWorkablePeople(int fromAge, int femaleToAge, | |||
| * return the names of all cats whose owners are women from `femaleAge` years old inclusively. | |||
| */ | |||
| public List<String> getCatsNames(List<Person> peopleList, int femaleAge) { | |||
| return Collections.emptyList(); | |||
| return peopleList.stream() | |||
| .filter(p -> p.getSex().equals(Person.Sex.WOMAN) && p.getAge() >= femaleAge) | |||
| .filter(candidateValidator) | ||
| .map(Candidate::getName) | ||
| .sorted() | ||
| .collect(Collectors.toList()); |
There was a problem hiding this comment.
| .collect(Collectors.toList()); | |
| .toList(); |
| .filter(p -> p.getSex().equals(Person.Sex.WOMAN) && p.getAge() >= femaleAge) | ||
| .flatMap(person -> person.getCats().stream()) | ||
| .map(Cat::getName) | ||
| .collect(Collectors.toList()); |
There was a problem hiding this comment.
| .collect(Collectors.toList()); | |
| .toList(); |
| && p.getAge() >= fromAge && p.getAge() <= maleToAge) | ||
| || p.getSex().equals(Person.Sex.WOMAN) | ||
| && p.getAge() >= fromAge && p.getAge() <= femaleToAge) | ||
| .collect(Collectors.toList()); |
There was a problem hiding this comment.
| .collect(Collectors.toList()); | |
| .toList(); |
rename p to person
| if (candidate.getAge() < PRESIDENT_ELIGIBILITY_AGE) { | ||
| return false; | ||
| } | ||
|
|
||
| if (!candidate.isAllowedToVote()) { | ||
| return false; | ||
| } | ||
|
|
||
| if (!"Ukrainian".equals(candidate.getNationality())) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Let’s combine conditional (see Olena comment)
| return false; | ||
| } | ||
|
|
||
| if (!"Ukrainian".equals(candidate.getNationality())) { |
There was a problem hiding this comment.
make nationality constant field
| return false; | ||
| } | ||
|
|
||
| String[] periods = candidate.getPeriodsInUkr().split(","); |
There was a problem hiding this comment.
make separator constant field
| String[] periods = candidate.getPeriodsInUkr().split(","); | ||
| int totalYearInUkraine = Arrays.stream(periods) | ||
| .mapToInt(period -> { | ||
| String[] range = period.trim().split("-"); |
| String[] range = period.trim().split("-"); | ||
| int startYear = Integer.parseInt(range[0]); | ||
| int endYear = Integer.parseInt(range[1]); | ||
| return endYear - startYear + 1; |
There was a problem hiding this comment.
| return endYear - startYear + 1; | |
| return endYear - startYear; |
let's make sure that this is the number of full years
| }) | ||
| .sum(); | ||
|
|
||
| return totalYearInUkraine >= 10; |
There was a problem hiding this comment.
Make 10 a constant field with an informative name
| .flatMap(str -> Arrays.stream(str.split(","))) | ||
| .map(Integer::parseInt) | ||
| .filter(num -> num % 2 == 0) | ||
| .min(Integer::compareTo) |
There was a problem hiding this comment.
| .min(Integer::compareTo) | |
| .min() |
It’s default behavior
| .map(i -> i % 2 == 1 ? numbers.get(i) - 1 : numbers.get(i)) | ||
| .filter(i -> i % 2 == 1) | ||
| .average() | ||
| .orElseThrow(NoSuchElementException::new); |
There was a problem hiding this comment.
Let’s provide an informative message
| .filter(person -> (person.getSex().equals(Person.Sex.MAN) | ||
| && person.getAge() >= fromAge && person.getAge() <= maleToAge) | ||
| || person.getSex().equals(Person.Sex.WOMAN) | ||
| && person.getAge() >= fromAge && person.getAge() <= femaleToAge) |
There was a problem hiding this comment.
for complicated conditional, it is better to use Predicate
re-work predicate interface in CandidateValidator
No description provided.