implemented helpers for elections#1405
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your implementation has correct logic for all tasks, but there are two explicit checklist violations that must be fixed: 1) Enum comparisons using == instead of equals() in StreamPractice.java for Tasks 3, 4, and 5 - the checklist explicitly requires using equals() method for enum comparisons. Change p.getSex() == Person.Sex.MAN to p.getSex().equals(Person.Sex.MAN) and similar for other enum comparisons. 2) Magic numbers and strings in CandidateValidator.java - replace 35 with a named constant like MIN_AGE, replace "Ukrainian" with a constant like REQUIRED_NATIONALITY, and replace 10 with MIN_YEARS_IN_UKRAINE. The checklist specifically states to use constants for magic numbers and strings. These are not cosmetic issues - they are explicit requirements that need to be addressed.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| int yearTo = Integer.parseInt(c.getPeriodsInUkr().split("-")[1]); | ||
| int yearsDiff = yearTo - yearFrom; | ||
|
|
||
| return c.getAge() >= 35 && c.isAllowedToVote() && c.getNationality().equals("Ukrainian") |
There was a problem hiding this comment.
Magic number 35 should be replaced with a named constant (e.g., MIN_AGE). The checklist requires using constants for magic numbers and strings in CandidateValidator.
| int yearTo = Integer.parseInt(c.getPeriodsInUkr().split("-")[1]); | ||
| int yearsDiff = yearTo - yearFrom; | ||
|
|
||
| return c.getAge() >= 35 && c.isAllowedToVote() && c.getNationality().equals("Ukrainian") |
There was a problem hiding this comment.
Magic string "Ukrainian" should be replaced with a named constant. The checklist requires using constants for magic numbers and strings in CandidateValidator.
| int yearsDiff = yearTo - yearFrom; | ||
|
|
||
| return c.getAge() >= 35 && c.isAllowedToVote() && c.getNationality().equals("Ukrainian") | ||
| && yearsDiff >= 10; |
There was a problem hiding this comment.
Magic number 10 should be replaced with a named constant (e.g., MIN_YEARS_IN_UKRAINE). The checklist requires using constants for magic numbers and strings in CandidateValidator.
| .filter(p -> { | ||
| return p.getSex() == Person.Sex.MAN |
There was a problem hiding this comment.
This uses == to compare Enum values. The checklist explicitly states to use equals() for comparing Enum values (not ==). Change to p.getSex().equals(Person.Sex.MAN)
| .filter(p -> { | ||
| int toAge = p.getSex() == Person.Sex.MAN ? maleToAge : femaleToAge; |
There was a problem hiding this comment.
This uses == to compare Enum values. The checklist explicitly states to use equals() for comparing Enum values (not ==). Change to p.getSex().equals(Person.Sex.MAN)
| .filter(p -> { | ||
| return p.getSex() == Person.Sex.WOMAN && p.getAge() >= femaleAge; |
There was a problem hiding this comment.
This uses == to compare Enum values. The checklist explicitly states to use equals() for comparing Enum values (not ==). Change to p.getSex().equals(Person.Sex.WOMAN)
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your solution is approved. All six tasks are correctly implemented using Stream API without explicit loops. The enum comparison issues from Tasks 3, 4, and 5 have been fixed—StreamPractice.java now properly uses equals() for Sex enum comparisons (lines 54, 75, 89). Additionally, CandidateValidator.java correctly replaces all magic numbers and strings with named constants (MIN_AGE, NATIONALITY, MIN_YEARS_IN_UKR), and implements Predicate<Candidate> as required. Great work addressing all previous checklist violations!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
3 participants
No description provided.