added worked code#1413
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your solution requires changes due to two critical issues. First, the findMinEvenNumber method will throw a NumberFormatException when processing inputs with whitespace, such as "0, 22, 7". The split(",") call preserves leading spaces, so " 22" cannot be parsed by Integer.parseInt(). You need to add .map(String::trim) before parsing. Second, enum comparisons throughout the code use == instead of equals(), which violates [CHECKLIST ITEM #5]. While == works for enums in Java, the checklist explicitly requires using .equals() for enum comparisons. Additionally, consider adding parentheses around compound OR conditions in getWorkablePeople and selectByWorkingAge for clarity, even though the current precedence happens to work correctly.
✨ 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
| return peopleList.stream() | ||
| .filter(person -> person.getAge() >= fromAge | ||
| && person.getAge() <= toAge && person.getSex() == Person.Sex.MAN) |
There was a problem hiding this comment.
This violates checklist item #5: 'Pay attention to what is a better way to compare Enum values: equals() vs == ?' Use .equals() instead of == for comparing enum values.
| return peopleList.stream().filter(person -> person.getSex() == Person.Sex.WOMAN | ||
| && person.getAge() >= fromAge && person.getAge() <= femaleToAge | ||
| || person.getSex() == Person.Sex.MAN |
There was a problem hiding this comment.
This violates checklist item #5: 'Pay attention to what is a better way to compare Enum values: equals() vs == ?' Use .equals() instead of == for comparing enum values.
| .filter(person -> person.getAge() >= femaleAge | ||
| && person.getSex() == Person.Sex.WOMAN) |
There was a problem hiding this comment.
This violates checklist item #5: 'Pay attention to what is a better way to compare Enum values: equals() vs == ?' Use .equals() instead of == for comparing enum values.
| .average() | ||
| .getAsDouble(); |
There was a problem hiding this comment.
The task requires throwing NoSuchElementException when no result is found, but .getAsDouble() throws java.util.NoSuchElementException. Consider using .orElseThrow(() -> new NoSuchElementException()) instead.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I'm requesting changes to address one critical checklist violation that remains in your code.
Critical Issue - Enum Comparison (CHECKLIST ITEM #5):
The getCatsNames() method in StreamPractice.java still uses == for enum comparison at line 87: person.getSex() == Person.Sex.WOMAN. While this technically works in Java (due to enum caching), the checklist explicitly requires using .equals() method for all enum comparisons. You've already demonstrated you understand this pattern correctly—selectMenByAge() and getWorkablePeople() both use .equals() properly. Please apply the same approach in getCatsNames().
Minor Improvement:
The getWorkablePeople() method's compound OR condition would benefit from additional parentheses for clarity. While Java's operator precedence would evaluate && before || correctly, wrapping each gender's condition in parentheses improves readability and prevents potential misunderstandings: (person.getSex().equals(Person.Sex.WOMAN) && ... && ...) || (person.getSex().equals(Person.Sex.MAN) && ... && ...).
You've made solid progress on this assignment—the whitespace handling fix in findMinEvenNumber shows good attention to detail. Once you update the enum comparison in getCatsNames(), your solution should fully meet the requirements.
✨ 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
| return peopleList.stream() | ||
| .filter(person -> person.getAge() >= femaleAge | ||
| && person.getSex() == Person.Sex.WOMAN) |
There was a problem hiding this comment.
This violates [CHECKLIST ITEM #5] - enum comparisons must use .equals() method instead of ==. Change person.getSex() == Person.Sex.WOMAN to person.getSex().equals(Person.Sex.WOMAN).
| return peopleList.stream().filter(person -> person.getSex().equals(Person.Sex.WOMAN) | ||
| && person.getAge() >= fromAge && person.getAge() <= femaleToAge | ||
| || person.getSex().equals(Person.Sex.MAN) | ||
| && person.getAge() >= fromAge && person.getAge() <= maleToAge) |
There was a problem hiding this comment.
Consider adding parentheses around the compound OR condition for better readability: (person.getSex().equals(Person.Sex.WOMAN) && person.getAge() >= fromAge && person.getAge() <= femaleToAge) || (person.getSex().equals(Person.Sex.MAN) && person.getAge() >= fromAge && person.getAge() <= maleToAge).
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 with an educational override. Great job fixing the enum comparison issue in getCatsNames() by using .equals() — that's a critical fix! All core requirements are implemented correctly with proper Stream API usage. There's one remaining checklist item in getWorkablePeople(): the compound OR conditions need explicit parentheses like (A && B && C) || (D && E && F) for better readability and to prevent potential logical errors, even though Java's precedence would technically evaluate it correctly. This is a minor readability concern that won't cause functional issues, but explicit grouping is considered best practice for complex boolean expressions. Keep up the excellent work on your learning journey!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| return peopleList.stream().filter(person -> person.getSex().equals(Person.Sex.WOMAN) | ||
| && person.getAge() >= fromAge && person.getAge() <= femaleToAge | ||
| || person.getSex().equals(Person.Sex.MAN) | ||
| && person.getAge() >= fromAge && person.getAge() <= maleToAge) |
There was a problem hiding this comment.
Missing parentheses around compound OR conditions. Due to Java operator precedence, && binds tighter than ||, which may cause incorrect filtering. Each gender's condition should be wrapped in parentheses to ensure correct logical evaluation: (person.getSex().equals(Person.Sex.WOMAN) && ... && ...) || (person.getSex().equals(Person.Sex.MAN) && ... && ...)
2 participants
No description provided.