passed checkstyle and tests#1416
Conversation
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving your solution because all core tasks are implemented correctly and the remaining issues are about clarity and style rather than broken functionality. Your stream usage, exception handling (including the required RuntimeException message in findMinEvenNumber), and the CandidateValidator predicate all meet the task requirements, and the business logic for filtering and averaging appears correct. The two points to improve are: in StreamPractice.getWorkablePeople (and similarly in CandidateValidator if you reuse that pattern), add parentheses to group the male and female conditions separately so the &&/|| combination is unambiguous and easier to read; and in StreamPractice.validateCandidates, move .filter(validator) onto a new line after .stream() to follow the “one stream method per line” guideline [CHECKLIST ITEM #5]. For example:
return people.stream()
.filter(person -> (person.getSex() == Person.Sex.MAN
&& person.getAge() >= fromAge
&& person.getAge() <= maleToAge)
|| (person.getSex() == Person.Sex.WOMAN
&& person.getAge() >= fromAge
&& person.getAge() <= femaleToAge))
.collect(Collectors.toList());Functionally, though, your current code satisfies the assignment, and these adjustments would primarily improve readability and alignment with the checklist. Nicely done overall on using streams consistently and encapsulating complex logic in a dedicated validator class.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| .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.
The compound predicate mixes && and || without parentheses, so the male and female conditions can be evaluated in an unintended order; to avoid a logic bug and align with the boolean-operator checklist item, explicitly group the male block and the female block with parentheses.
| public List<String> validateCandidates(List<Candidate> candidates) { | ||
| return Collections.emptyList(); | ||
| CandidateValidator validator = new CandidateValidator(); | ||
| return candidates.stream().filter(validator) |
There was a problem hiding this comment.
Checklist item #5 says: "Call each new method from a new line in the stream"; here .stream() and .filter(validator) are on the same line, so move .filter(validator) to its own line in the chain.
No description provided.