Hw 14 jv stream practice #1197
Hw 14 jv stream practice #1197
Conversation
There was a problem hiding this comment.
Great job on implementing the core functionality and meeting most of the task requirements! 🎉 While there are a few areas for improvement, such as enhancing exception handling and clarifying certain logic, these are minor and do not significantly impact the overall functionality of your code. Keep up the good work, and consider the feedback to make your code even more robust and maintainable in the future. 😊
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
|
|
||
| @Override | ||
| public boolean test(Candidate candidate) { | ||
| String[] years = candidate.getPeriodsInUkr().split("-"); |
There was a problem hiding this comment.
Ensure that the getPeriodsInUkr() method in the Candidate class returns a valid string in the format 'YYYY-YYYY'. If the format is incorrect, ArrayIndexOutOfBoundsException or NumberFormatException might occur.
| int fromYear = Integer.parseInt(years[0]); | ||
| int toYear = Integer.parseInt(years[1]); |
There was a problem hiding this comment.
Consider adding error handling for NumberFormatException in case the years cannot be parsed into integers. This will make the code more robust against invalid input.
| .orElseThrow(() -> new RuntimeException("Can't get min value from list: " | ||
| + numbers)); |
There was a problem hiding this comment.
The use of RuntimeException in orElseThrow is acceptable, but consider using a more specific exception type or a custom exception to provide clearer context about the error.
| return IntStream.range(0, numbers.size()) | ||
| .map(i -> (i % 2 != 0) ? numbers.get(i) - 1 : numbers.get(i)) |
There was a problem hiding this comment.
The logic for adjusting numbers at odd indices by subtracting 1 is not immediately clear. Ensure this logic is intentional and documented, as it may confuse future maintainers.
| .map(i -> (i % 2 != 0) ? numbers.get(i) - 1 : numbers.get(i)) | ||
| .filter(n -> n % 2 != 0) | ||
| .average() | ||
| .orElseThrow(NoSuchElementException::new); |
There was a problem hiding this comment.
Throwing NoSuchElementException is appropriate here, but consider providing a custom message to clarify the context of the exception.
|
|
||
| @Override | ||
| public boolean test(Candidate candidate) { | ||
| String[] years = candidate.getPeriodsInUkr().split("-"); |
There was a problem hiding this comment.
better make separator constant field
| .sorted() | ||
| .findFirst() |
There was a problem hiding this comment.
you can replace it using min()
| .filter(p -> p.getSex() == Person.Sex.MAN | ||
| && p.getAge() >= fromAge | ||
| && p.getAge() <= toAge) | ||
| .collect(Collectors.toList()); |
There was a problem hiding this comment.
| .collect(Collectors.toList()); | |
| .toList(); |
Let’s fix it everywhere
No description provided.