done #1215

abeshle · 2025-01-09T03:26:02Z

No description provided.

mateacademy-ai-mentor

Thank you for your hard work on this pull request! 😊 While your code meets most of the task requirements, there are a few critical issues that need to be addressed before we can approve it:

Age Comparison Logic: In the selectMenByAge method, ensure that the age comparison is inclusive for both fromAge and toAge. Change p.getAge() > fromAge to p.getAge() >= fromAge to correctly include the lower bound as specified in the task description.
Exception Message Formatting: The exception message in the findMinEvenNumber method should include a space after the colon, as specified in the checklist: "Can't get min value from list: " + numbers.
Use of Constants: In the CandidateValidator class, replace magic numbers and strings with constants to enhance readability and maintainability. Specifically, consider using constants for:
- The minimum years lived in Ukraine (10).
- The nationality string ('Ukrainian').
- The age requirement (35).

Addressing these issues will help ensure your code is both functional and maintainable. Keep up the great work, and I'm confident you'll nail it with these adjustments! 🚀

✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨

🔄 Fix the issues, commit the changes, and then re-request my review using the Re-request review button in the Reviewers tab.

mateacademy-ai-mentor · 2025-01-09T03:27:11Z

src/main/java/practice/CandidateValidator.java

+
+    @Override
+    public boolean test(Candidate candidate) {
+        if (candidate.getAge() < 35) {


Consider using a constant for the age requirement (35) to improve code readability and maintainability. This aligns with the checklist item: 'Use constants. Magic numbers and strings decrease code readability, let's avoid it and use constants in class CandidateValidator.'

mateacademy-ai-mentor · 2025-01-09T03:27:11Z

src/main/java/practice/CandidateValidator.java

+        if (candidate.getAge() < 35) {
+            return false;
+        }
+        if (!(candidate.getNationality().equals("Ukrainian"))) {


Consider using a constant for the nationality string 'Ukrainian' to improve code readability and maintainability. This aligns with the checklist item: 'Use constants. Magic numbers and strings decrease code readability, let's avoid it and use constants in class CandidateValidator.'

mateacademy-ai-mentor · 2025-01-09T03:27:11Z

src/main/java/practice/CandidateValidator.java

+        }
+        int start = Integer.parseInt(periods[0]);
+        int finish = Integer.parseInt(periods[1]);
+        return (finish - start) >= 10;


Consider using a constant for the minimum years lived in Ukraine (10) to improve code readability and maintainability. This aligns with the checklist item: 'Use constants. Magic numbers and strings decrease code readability, let's avoid it and use constants in class CandidateValidator.'

mateacademy-ai-mentor · 2025-01-09T03:27:11Z

src/main/java/practice/StreamPractice.java

+                .sorted()
+                .mapToInt(Integer::intValue)
+                .findFirst()
+                .orElseThrow(() -> new RuntimeException("Can't get min value from list:"


The exception message should include a space after the colon, as specified in the checklist: "Can't get min value from list: " + numbers.

mateacademy-ai-mentor · 2025-01-09T03:27:11Z

src/main/java/practice/StreamPractice.java

-        return Collections.emptyList();
+        return peopleList.stream()
+                .filter(p -> p.getSex() == Person.Sex.MAN
+                        && p.getAge() > fromAge && p.getAge() <= toAge)


The age comparison should be inclusive for both fromAge and toAge. Change p.getAge() > fromAge to p.getAge() >= fromAge to include the lower bound as specified in the task description.

mateacademy-ai-mentor

Great job on addressing the previous issues! 🎉 The CandidateValidator class now uses constants, enhancing readability and maintainability. The StreamPractice class has been improved with correct age comparison logic and exception message formatting. All critical issues have been resolved, and the code meets the task requirements. Keep up the good work and continue learning from the feedback to further improve your coding skills! 🚀

✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨

liliia-ponomarenko

Good job! Let’s improve your solution ;)

liliia-ponomarenko · 2025-01-10T08:15:12Z

src/main/java/practice/CandidateValidator.java

+    private final String nationality = "Ukrainian";
+    private final int age = 35;
+    private final int minYearsInUkraine = 10;


static missed. And use the correct name format: NATIONALITY...

liliia-ponomarenko · 2025-01-10T08:16:16Z

src/main/java/practice/CandidateValidator.java

+        if (candidate.getAge() < age) {
+            return false;
+        }
+        if (!(candidate.getNationality().equals(nationality))) {
+            return false;
+        }
+        if (!candidate.isAllowedToVote()) {
+            return false;
+        }


Can you combine these conditionals?

liliia-ponomarenko · 2025-01-10T08:16:30Z

src/main/java/practice/CandidateValidator.java

+        if (!candidate.isAllowedToVote()) {
+            return false;
+        }
+        String[]periods = candidate.getPeriodsInUkr().split("-");


make separator constant field

liliia-ponomarenko · 2025-01-10T08:16:35Z

src/main/java/practice/CandidateValidator.java

+        int start = Integer.parseInt(periods[0]);
+        int finish = Integer.parseInt(periods[1]);
+        return (finish - start) >= minYearsInUkraine;
+


Suggested change

liliia-ponomarenko · 2025-01-10T08:16:52Z

src/main/java/practice/StreamPractice.java

+
+        return numbers.stream()
+                .flatMap(number -> Arrays.stream(number.split(",")))
+                .map(String::trim)


Suggested change

.map(String::trim)

liliia-ponomarenko · 2025-01-10T08:17:38Z

src/main/java/practice/StreamPractice.java

+                .sorted()
+                .mapToInt(Integer::intValue)
+                .findFirst()


use the min function

liliia-ponomarenko · 2025-01-10T08:18:03Z

src/main/java/practice/StreamPractice.java

+                .map(i -> (i % 2 != 0) ? numbers.get(i) - 1 : numbers.get(i))
+                .filter(n -> n % 2 != 0)
+                .average()
+                .orElseThrow(() -> new NoSuchElementException());


add informative message

liliia-ponomarenko · 2025-01-10T08:18:27Z

src/main/java/practice/StreamPractice.java

+        return peopleList.stream()
+                .filter(p -> p.getSex() == Person.Sex.MAN
+                        && p.getAge() >= fromAge && p.getAge() <= toAge)
+                .collect(Collectors.toList());


Suggested change

.collect(Collectors.toList());

.toList();

fix it everywhere

liliia-ponomarenko · 2025-01-10T08:18:54Z

src/main/java/practice/StreamPractice.java

+                .filter(p -> (p.getSex() == Person.Sex.MAN
+                        && p.getAge() >= fromAge && p.getAge() <= maleToAge)
+                        || (p.getSex() == Person.Sex.WOMAN && p.getAge() >= fromAge
+                        && p.getAge() <= femaleToAge))


for complicated conditional better use Predicate

done

7cbbcaf

mateacademy-ai-mentor suggested changes Jan 9, 2025

View reviewed changes

made changes

42905c8

abeshle requested a review from mateacademy-ai-mentor January 10, 2025 01:18

mateacademy-ai-mentor approved these changes Jan 10, 2025

View reviewed changes

liliia-ponomarenko suggested changes Jan 10, 2025

View reviewed changes

Improved solution according to latest request

c99f9e6

abeshle requested review from liliia-ponomarenko and mateacademy-ai-mentor January 11, 2025 03:20

Elena-Bruyako approved these changes Jan 11, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

done #1215

done #1215

abeshle commented Jan 9, 2025

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor Jan 9, 2025

mateacademy-ai-mentor Jan 9, 2025

mateacademy-ai-mentor Jan 9, 2025

mateacademy-ai-mentor Jan 9, 2025

mateacademy-ai-mentor Jan 9, 2025

mateacademy-ai-mentor left a comment

liliia-ponomarenko left a comment

liliia-ponomarenko Jan 10, 2025

liliia-ponomarenko Jan 10, 2025

liliia-ponomarenko Jan 10, 2025

liliia-ponomarenko Jan 10, 2025

liliia-ponomarenko Jan 10, 2025

liliia-ponomarenko Jan 10, 2025

liliia-ponomarenko Jan 10, 2025

liliia-ponomarenko Jan 10, 2025

liliia-ponomarenko Jan 10, 2025

done #1215

Are you sure you want to change the base?

done #1215

Conversation

abeshle commented Jan 9, 2025

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

liliia-ponomarenko left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment