implement all methods #1220

FroGitHub · 2025-01-17T16:52:45Z

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:

Handling Null or Empty Lists: It's important to check if lists like candidates, peopleList, and numbers are null or empty before processing them. This will help avoid potential NullPointerException errors and ensure your code is robust.
Use of Constants: To improve readability and maintainability, consider using constants for magic numbers and strings, such as the minimum residency period, age requirement, and nationality string. This aligns with best practices and will make your code easier to understand and modify in the future.
Exception Message Format: Ensure that the exception message in the findMinEvenNumber method is formatted correctly as per the task description.

Addressing these issues will enhance the functionality and quality of your code. Keep up the great work, and I look forward to seeing your improvements! 😊

✨ 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-17T16:53:55Z

src/main/java/practice/CandidateValidator.java

+        if (candidate == null) {
+            return false;
+        }
+        if (!"Ukrainian".equals(candidate.getNationality())) {


Consider using a constant for the nationality string "Ukrainian" to improve readability and maintainability. This aligns with the checklist advice to avoid magic strings.

mateacademy-ai-mentor · 2025-01-17T16:53:55Z

src/main/java/practice/CandidateValidator.java

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


Consider using a constant for the age requirement (35) to improve readability and maintainability. This aligns with the checklist advice to avoid magic numbers.

mateacademy-ai-mentor · 2025-01-17T16:53:55Z

src/main/java/practice/CandidateValidator.java

+                    .map(Integer::valueOf)
+                    .collect(Collectors.toList());
+            if (periodInUkraine.size() != 2
+                    || periodInUkraine.get(1) - periodInUkraine.get(0) < 10) {


Consider using a constant for the minimum residency period (10 years) to improve readability and maintainability. This aligns with the checklist advice to avoid magic numbers.

mateacademy-ai-mentor · 2025-01-17T16:53:56Z

src/main/java/practice/StreamPractice.java

@@ -14,7 +18,16 @@ public class StreamPractice {
     * "Can't get min value from list: < Here is our input 'numbers' >"
     */
    public int findMinEvenNumber(List<String> numbers) {
-        return 0;
+        return numbers.stream()


Consider checking if the numbers list is null or empty before processing to avoid potential NullPointerException.

mateacademy-ai-mentor · 2025-01-17T16:53:56Z

src/main/java/practice/StreamPractice.java

@@ -23,7 +36,12 @@ public int findMinEvenNumber(List<String> numbers) {
     * But before that subtract 1 from each element on an odd position (having the odd index).
     */
    public Double getOddNumsAverage(List<Integer> numbers) {


Consider checking if the numbers list is null or empty before processing to avoid potential NullPointerException.

mateacademy-ai-mentor · 2025-01-17T16:53:56Z

src/main/java/practice/StreamPractice.java

@@ -35,7 +53,10 @@ public Double getOddNumsAverage(List<Integer> numbers) {
     * Example: select men who can be recruited to army (from 18 to 27 years old inclusively).
     */
    public List<Person> selectMenByAge(List<Person> peopleList, int fromAge, int toAge) {


Consider checking if the peopleList is null or empty before processing to avoid potential NullPointerException.

mateacademy-ai-mentor · 2025-01-17T16:53:56Z

src/main/java/practice/StreamPractice.java

@@ -50,7 +71,11 @@ public List<Person> selectMenByAge(List<Person> peopleList, int fromAge, int toA
     */
    public List<Person> getWorkablePeople(int fromAge, int femaleToAge,


Consider checking if the peopleList is null or empty before processing to avoid potential NullPointerException.

mateacademy-ai-mentor · 2025-01-17T16:53:56Z

src/main/java/practice/StreamPractice.java

@@ -59,10 +84,15 @@ public List<Person> getWorkablePeople(int fromAge, int femaleToAge,
     * return the names of all cats whose owners are women from `femaleAge` years old inclusively.
     */
    public List<String> getCatsNames(List<Person> peopleList, int femaleAge) {


Consider checking if the peopleList is null or empty before processing to avoid potential NullPointerException.

mateacademy-ai-mentor · 2025-01-17T16:53:56Z

src/main/java/practice/StreamPractice.java

@@ -75,6 +105,10 @@ public List<String> getCatsNames(List<Person> peopleList, int femaleAge) {
     * parametrized with Candidate in CandidateValidator.
     */
    public List<String> validateCandidates(List<Candidate> candidates) {


Consider checking if the candidates list is null or empty before processing to avoid potential NullPointerException.

mateacademy-ai-mentor

Great job on addressing the issues from the previous review! 🎉 Your code now meets the task requirements and constraints. Here are some key points to consider for further improvement:

Use of Constants: You've correctly defined constants for neededAge and neededPeriod, but remember to follow Java naming conventions by using uppercase (e.g., NEEDED_AGE, NEEDED_PERIOD).
Handling Null or Empty Lists: You've added checks for null or empty lists in various methods, which is excellent. However, consider throwing a NoSuchElementException instead of returning default values like 0D to maintain consistency across your methods.
Exception Handling: The exception message in the findMinEvenNumber method is now formatted correctly. Ensure that you validate the format of strings like periodsInUkr before parsing to improve exception handling and avoid potential errors.

Your efforts to improve the code are commendable. Keep up the good work, and make sure to address these points in future projects to enhance your coding skills further. Remember, learning from feedback is a crucial part of your development journey. 🚀

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

mateacademy-ai-mentor · 2025-01-17T17:05:09Z

src/main/java/practice/CandidateValidator.java

+    private static final int neededAge = 35;
+    private static final int neededPeriod = 10;


The constants neededAge and neededPeriod should be named in uppercase (e.g., NEEDED_AGE, NEEDED_PERIOD) to follow Java naming conventions for constants.

fix comment

mateacademy-ai-mentor · 2025-01-17T17:05:10Z

src/main/java/practice/CandidateValidator.java

+            List<Integer> periodInUkraine = Arrays.stream(candidate.getPeriodsInUkr().split("-"))
+                    .map(String::trim)
+                    .map(Integer::valueOf)
+                    .collect(Collectors.toList());


Before splitting and processing the periodsInUkr string, ensure it is not null or empty to avoid potential exceptions.

mateacademy-ai-mentor · 2025-01-17T17:05:10Z

src/main/java/practice/CandidateValidator.java

+        } catch (NumberFormatException | IndexOutOfBoundsException e) {
+            return false;


Consider validating the format of the periodsInUkr string before parsing to integers to improve exception handling.

mateacademy-ai-mentor · 2025-01-17T17:05:10Z

src/main/java/practice/StreamPractice.java

@@ -14,7 +19,19 @@ public class StreamPractice {
     * "Can't get min value from list: < Here is our input 'numbers' >"
     */
    public int findMinEvenNumber(List<String> numbers) {
-        return 0;
+        if (numbers == null || numbers.isEmpty()) {
+            throw new RuntimeException("Can't get min value from list");


The exception message should include the input list as specified in the task description: "Can't get min value from list: < Here is our input 'numbers' >".

mateacademy-ai-mentor · 2025-01-17T17:05:10Z

src/main/java/practice/StreamPractice.java

+        if (numbers == null || numbers.isEmpty()) {
+            return 0D;


Consider throwing a NoSuchElementException instead of returning 0D when the list is null or empty, to maintain consistency with other methods that throw exceptions when no valid data is found.

Elena-Bruyako · 2025-01-18T17:49:19Z

src/main/java/practice/CandidateValidator.java

+    private static final int neededAge = 35;
+    private static final int neededPeriod = 10;


fix comment

Elena-Bruyako · 2025-01-18T17:49:29Z

src/main/java/practice/CandidateValidator.java

+        if (candidate == null) {
+            return false;
+        }
+        if (!candidate.getNationality().equals("Ukrainian")) {


create constant for "Ukrainian"

Elena-Bruyako · 2025-01-18T17:51:12Z

src/main/java/practice/CandidateValidator.java

+
+    @Override
+    public boolean test(Candidate candidate) {
+        if (candidate == null) {


can you simplify solution by using logical operators?

Elena-Bruyako · 2025-01-18T17:52:29Z

src/main/java/practice/StreamPractice.java

+        }
+        return numbers.stream()
+                .map(s -> s.split(","))
+                .flatMap(Arrays::stream)


combine into one map

Elena-Bruyako · 2025-01-18T17:52:36Z

src/main/java/practice/StreamPractice.java

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


Elena-Bruyako · 2025-01-18T17:53:07Z

src/main/java/practice/StreamPractice.java

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


for all places

Suggested change

.collect(Collectors.toList());

.toList();

fix comment

Elena-Bruyako · 2025-01-18T17:53:21Z

src/main/java/practice/StreamPractice.java

@@ -14,7 +19,19 @@ public class StreamPractice {
     * "Can't get min value from list: < Here is our input 'numbers' >"
     */
    public int findMinEvenNumber(List<String> numbers) {
-        return 0;
+        if (numbers == null || numbers.isEmpty()) {


remove (for all other methods also)

Elena-Bruyako · 2025-01-19T09:36:56Z

src/main/java/practice/CandidateValidator.java

+import model.Candidate;
+
+public class CandidateValidator implements Predicate<Candidate> {
+    // National for being candidate


remove comments

Elena-Bruyako · 2025-01-19T09:37:09Z

src/main/java/practice/CandidateValidator.java

+
+public class CandidateValidator implements Predicate<Candidate> {
+    // National for being candidate
+    private static final String neededNational = "Ukrainian";


check naming convention for constants

Elena-Bruyako · 2025-01-19T09:37:56Z

src/main/java/practice/StreamPractice.java

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


fix comment

Elena-Bruyako · 2025-01-19T09:38:02Z

src/main/java/practice/StreamPractice.java

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


Elena-Bruyako · 2025-01-19T09:38:05Z

src/main/java/practice/StreamPractice.java

-        return Collections.emptyList();
+
+        return peopleList.stream()
+                .filter(p -> p.getAge() >= fromAge)


Elena-Bruyako · 2025-01-19T09:38:26Z

src/main/java/practice/StreamPractice.java

    }

-    /**
+    /**               V - wtf MA, wrong article


revert changes

Elena-Bruyako · 2025-01-19T09:38:33Z

src/main/java/practice/StreamPractice.java

+                .filter(p -> p.getAge() >= femaleAge)
+                .flatMap(c -> c.getCats().stream())
+                .map(Cat::getName)
+                .collect(Collectors.toList());


Suggested change

.collect(Collectors.toList());

.toList();

Elena-Bruyako · 2025-01-19T09:38:42Z

src/main/java/practice/StreamPractice.java

+                .filter(new CandidateValidator())
+                .map(Candidate::getName)
+                .sorted()
+                .collect(Collectors.toList());


Suggested change

.collect(Collectors.toList());

.toList();

Elena-Bruyako · 2025-01-19T09:38:51Z

src/main/java/practice/StreamPractice.java

+        return peopleList.stream()
+                .filter(p -> p.getSex() == Person.Sex.WOMAN)
+                .filter(p -> p.getAge() >= femaleAge)
+                .flatMap(c -> c.getCats().stream())


Elena-Bruyako · 2025-01-19T09:39:13Z

src/main/java/practice/StreamPractice.java

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


Suggested change

.filter((p -> (p.getSex() == Person.Sex.WOMAN && p.getAge() <= femaleToAge)

|| (p.getSex() == Person.Sex.MAN && p.getAge() <= maleToAge)))

.filter(p -> (p.getSex() == Person.Sex.WOMAN && p.getAge() <= femaleToAge

|| p.getSex() == Person.Sex.MAN && p.getAge() <= maleToAge))

Elena-Bruyako

fix previous comments

Elena-Bruyako · 2025-01-19T11:25:09Z

src/main/java/practice/CandidateValidator.java

+import model.Candidate;
+
+public class CandidateValidator implements Predicate<Candidate> {
+    private static final String requirementNationality = "Ukrainian";


read about constant naming convention and rename

liliia-ponomarenko

Good job! Let’s improve your solution ;)

liliia-ponomarenko · 2025-01-20T08:42:28Z

src/main/java/practice/CandidateValidator.java

+        }
+        try {
+            List<Integer> periodInUkraine = Arrays.stream(candidate.getPeriodsInUkr().split("-"))
+                    .map(String::trim)


Suggested change

.map(String::trim)

liliia-ponomarenko · 2025-01-20T08:44:20Z

src/main/java/practice/CandidateValidator.java

+                        - periodInUkraine.get(0) < REQUIREMENT_PERIOD_IN_UKR) {
+                return false;
+            }
+        } catch (NumberFormatException | IndexOutOfBoundsException e) {


Suggested change

} catch (NumberFormatException | IndexOutOfBoundsException e) {

} catch (NumberFormatException e) {

you validated the collection size

liliia-ponomarenko · 2025-01-20T08:44:43Z

src/main/java/practice/CandidateValidator.java

+            return false;
+        }
+        try {
+            List<Integer> periodInUkraine = Arrays.stream(candidate.getPeriodsInUkr().split("-"))


make separator constant field

liliia-ponomarenko · 2025-01-20T08:45:06Z

src/main/java/practice/CandidateValidator.java

}
+

leave only one line at the end of the file

liliia-ponomarenko · 2025-01-20T08:45:38Z

src/main/java/practice/StreamPractice.java

+                .flatMap(s -> Arrays.stream(s.split(",")))
+                .map(Integer::valueOf)
+                .filter(i -> i % 2 == 0)
+                .min(Integer::compareTo)


Suggested change

.min(Integer::compareTo)

.min()

It is the default behavior

I got error of compilation :(

liliia-ponomarenko · 2025-01-20T08:46:01Z

src/main/java/practice/StreamPractice.java

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


liliia-ponomarenko · 2025-01-20T08:46:41Z

src/main/java/practice/StreamPractice.java

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


for complicated conditional, it is better to use Predicate

liliia-ponomarenko · 2025-01-20T08:47:21Z

src/main/java/practice/StreamPractice.java

-        return Collections.emptyList();
+
+        return candidates.stream()
+                .filter(new CandidateValidator())


better make CandidateValidator class-level variable

implement all methods

54356ef

mateacademy-ai-mentor suggested changes Jan 17, 2025

View reviewed changes

correcting my code

2d0550a

FroGitHub requested a review from mateacademy-ai-mentor January 17, 2025 17:03

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

View reviewed changes

Elena-Bruyako suggested changes Jan 18, 2025

View reviewed changes

correcting my code x2

9a44c01

FroGitHub requested a review from Elena-Bruyako January 19, 2025 09:34

Elena-Bruyako suggested changes Jan 19, 2025

View reviewed changes

correcting my code x3

9c2b0b4

FroGitHub requested a review from Elena-Bruyako January 19, 2025 10:18

Elena-Bruyako suggested changes Jan 19, 2025

View reviewed changes

FroGitHub added 2 commits January 19, 2025 13:28

changed the names of the constants

9a6b566

changed the names of the constants

b37ef4e

FroGitHub requested a review from Elena-Bruyako January 19, 2025 11:32

liliia-ponomarenko suggested changes Jan 20, 2025

View reviewed changes

improved my solution

be5b217

FroGitHub requested a review from liliia-ponomarenko January 20, 2025 11:35

Elena-Bruyako approved these changes Jan 20, 2025

View reviewed changes

		@@ -50,7 +71,11 @@ public List<Person> selectMenByAge(List<Person> peopleList, int fromAge, int toA
		*/
		public List<Person> getWorkablePeople(int fromAge, int femaleToAge,

		private static final int neededAge = 35;
		private static final int neededPeriod = 10;

		} catch (NumberFormatException \| IndexOutOfBoundsException e) {
		return false;

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

	} catch (NumberFormatException \| IndexOutOfBoundsException e) {
	} catch (NumberFormatException e) {

implement all methods #1220

Are you sure you want to change the base?

implement all methods #1220

Conversation

FroGitHub commented Jan 17, 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

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

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

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

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

Elena-Bruyako left a comment

Choose a reason for hiding this 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