f-hw-jv-stream-github-practice #1208

ihor-sydorenko · 2024-12-21T23:05:37Z

No description provided.

mateacademy-ai-mentor

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

Inclusivity in Age Filtering: The condition in selectMenByAge should include the lower bound fromAge as specified in the task description. Please change the condition to p.getAge() >= fromAge && p.getAge() <= toAge.
Exception Handling in getLivePeriodInUkraine: The method assumes the input format is always correct, which might lead to exceptions. Consider adding validation or exception handling to manage potential parsing issues.
Unnecessary Type Check: The check candidate.getClass().equals(Candidate.class) is redundant since the parameter is already of type Candidate. Removing this will clean up your code.

These changes are essential to ensure the functionality and robustness of your solution. Once you've addressed these points, your code will be in great shape! Keep up the good work, and don't hesitate to reach out if you have any questions. 😊

✨ 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 · 2024-12-21T23:06:27Z

src/main/java/practice/CandidateValidator.java

+
+    @Override
+    public boolean test(Candidate candidate) {
+        return candidate.getClass().equals(Candidate.class)


The check candidate.getClass().equals(Candidate.class) is unnecessary. Since the method parameter is already of type Candidate, this check will always be true. Consider removing it.

mateacademy-ai-mentor · 2024-12-21T23:06:27Z

src/main/java/practice/CandidateValidator.java

+                && getLivePeriodInUkraine(candidate.getPeriodsInUkr()) >= MIN_PERIOD_IN_UKRAINE;
+    }
+
+    public static int getLivePeriodInUkraine(String periodsInUkr) {


The method getLivePeriodInUkraine assumes that the input string periodsInUkr is always in the correct format (e.g., '2000-2010'). If the format is incorrect or if there are parsing issues, this could throw a NumberFormatException or ArrayIndexOutOfBoundsException. Consider adding validation or exception handling to manage such cases.

mateacademy-ai-mentor · 2024-12-21T23:06:27Z

src/main/java/practice/StreamPractice.java

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


The condition p.getAge() > fromAge && p.getAge() <= toAge should be p.getAge() >= fromAge && p.getAge() <= toAge to include the lower bound fromAge in the age range, as the task description specifies inclusivity.

mateacademy-ai-mentor · 2024-12-21T23:06:27Z

src/main/java/practice/StreamPractice.java

+                        || (person.getSex().equals(Person.Sex.WOMAN)
+                        && person.getAge() >= fromAge
+                        && person.getAge() <= femaleToAge)))
+                .toList();


The method toList() is used here, which is available from Java 16 onwards. If you are using an earlier version of Java, consider using collect(Collectors.toList()) instead.

mateacademy-ai-mentor · 2024-12-21T23:06:28Z

src/main/java/practice/StreamPractice.java

@@ -69,12 +102,16 @@ public List<String> getCatsNames(List<Person> peopleList, int femaleAge) {
     * names sorted alphabetically.
     * The requirements are: person should be older than 35 years, should be allowed to vote,
     * have nationality - 'Ukrainian'
-     * and live in Ukraine for 10 years. For the last requirement use field periodsInUkr,
+     * and live in Ukraine for 10 years. For the last requirement use field Ukrainian',


The comment seems to have a typo: 'Ukrainian' is mentioned twice. Ensure the comment accurately reflects the field name used in the Candidate class.

mateacademy-ai-mentor

Great job on implementing the CandidateValidator and StreamPractice classes! 🎉 Your solution is well-structured and mostly meets the task requirements. Here are a few points to consider for improvement:

Inclusivity in Age Filtering: In the getWorkablePeople method, ensure that the age condition for men includes the lower bound by using person.getAge() >= fromAge. This will align with the task description and ensure accurate filtering.
Input Validation in getLivePeriodInUkraine: The method currently assumes the input string is always correctly formatted. Adding validation to check the format before parsing can prevent potential exceptions or incorrect calculations.
Exception Handling: The current exception handling in getLivePeriodInUkraine only prints an error message. Consider handling exceptions more robustly, such as returning a default value or rethrowing the exception with a custom message, to enhance the robustness of your code.

You've made significant progress, and addressing these points will further strengthen your solution. Keep up the great work and continue learning from these experiences! 🚀

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

mateacademy-ai-mentor · 2024-12-22T21:11:53Z

src/main/java/practice/CandidateValidator.java

+        } catch (NumberFormatException e) {
+            System.out.println("String does not contain a parsable integer");


The current exception handling only prints an error message. Consider handling the exception more robustly, such as by returning a default value or rethrowing the exception with a custom message.

mateacademy-ai-mentor · 2024-12-22T21:11:53Z

src/main/java/practice/CandidateValidator.java

+    public static int getLivePeriodInUkraine(String periodsInUkr) {
+        String[] split = periodsInUkr.split("-");
+        int period = 0;
+        try {
+            period = Integer.parseInt(split[LAST_INDEX_END])
+                    - Integer.parseInt(split[FIRST_INDEX_START]);


The method getLivePeriodInUkraine assumes the input string is always in the correct format (e.g., 'YYYY-YYYY'). Consider adding validation to ensure the input format is correct before parsing, to prevent potential exceptions or incorrect calculations.

mateacademy-ai-mentor · 2024-12-22T21:11:53Z

src/main/java/practice/StreamPractice.java

+                        && person.getAge() > fromAge
+                        && person.getAge() <= maleToAge)


In the getWorkablePeople method, the condition person.getAge() > fromAge should be person.getAge() >= fromAge to include the lower bound age for men, as specified in the task description.

liliia-ponomarenko

Good job! Let’s improve your solution ;)

liliia-ponomarenko · 2024-12-23T07:30:55Z

src/main/java/practice/CandidateValidator.java

+    }
+
+    public static int getLivePeriodInUkraine(String periodsInUkr) {
+        String[] split = periodsInUkr.split("-");


it also can be a constant

liliia-ponomarenko · 2024-12-23T07:31:20Z

src/main/java/practice/CandidateValidator.java

+                && getLivePeriodInUkraine(candidate.getPeriodsInUkr()) >= MIN_PERIOD_IN_UKRAINE;
+    }
+
+    public static int getLivePeriodInUkraine(String periodsInUkr) {


Why public static?

Suggested change

public static int getLivePeriodInUkraine(String periodsInUkr) {

private int getLivePeriodInUkraine(String periodsInUkr) {

liliia-ponomarenko · 2024-12-23T07:33:11Z

src/main/java/practice/CandidateValidator.java

+            period = Integer.parseInt(split[LAST_INDEX_END])
+                    - Integer.parseInt(split[FIRST_INDEX_START]);
+        } catch (NumberFormatException e) {
+            System.out.println("String does not contain a parsable integer");


Let’s throw a runtime exception. Remove printing to the console. We'll explore logging a little later.

liliia-ponomarenko · 2024-12-23T07:33:49Z

src/main/java/practice/StreamPractice.java

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


Let’s add a message

liliia-ponomarenko · 2024-12-23T07:34:03Z

src/main/java/practice/StreamPractice.java

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


Suggested change

.collect(Collectors.toList());

.toList();

liliia-ponomarenko · 2024-12-23T07:34:47Z

src/main/java/practice/StreamPractice.java

@@ -50,7 +70,14 @@ public List<Person> selectMenByAge(List<Person> peopleList, int fromAge, int toA
     */
    public List<Person> getWorkablePeople(int fromAge, int femaleToAge,
                                          int maleToAge, List<Person> peopleList) {
-        return Collections.emptyList();
+        return peopleList.stream()
+                .filter(person -> ((person.getSex().equals(Person.Sex.MAN)


Suggested change

.filter(person -> ((person.getSex().equals(Person.Sex.MAN)

.filter(person -> ((person.getSex() == Person.Sex.MAN

Let’s compare enums in this way

liliia-ponomarenko · 2024-12-23T07:35:33Z

src/main/java/practice/StreamPractice.java

+                        || (person.getSex().equals(Person.Sex.WOMAN)
+                        && person.getAge() >= fromAge
+                        && person.getAge() <= femaleToAge)))


for complicated conditional, it is better to use Predicate

how I can do this? if fromAge, femaleToAge and maleToAge is a variable that we accept as input to the method. They are not constant.

liliia-ponomarenko · 2024-12-23T07:36:55Z

src/main/java/practice/StreamPractice.java

+                        || (person.getSex().equals(Person.Sex.WOMAN)
+                        && person.getAge() >= fromAge
+                        && person.getAge() <= femaleToAge)))
+                .collect(Collectors.toList());


Suggested change

.collect(Collectors.toList());

.toList();

fix it everywhere

liliia-ponomarenko · 2024-12-23T07:37:31Z

src/main/java/practice/StreamPractice.java

@@ -69,12 +102,16 @@ public List<String> getCatsNames(List<Person> peopleList, int femaleAge) {
     * names sorted alphabetically.
     * The requirements are: person should be older than 35 years, should be allowed to vote,
     * have nationality - 'Ukrainian'
-     * and live in Ukraine for 10 years. For the last requirement use field periodsInUkr,
+     * and live in Ukraine for 10 years. For the last requirement use field Ukrainian',


revert task description

liliia-ponomarenko

Good job! A few minor comments ;)

liliia-ponomarenko · 2024-12-25T08:19:55Z

src/main/java/practice/StreamPractice.java

@@ -35,7 +52,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) {
-        return Collections.emptyList();
+        return peopleList.stream()
+                .filter(p -> p.getSex().equals(Person.Sex.MAN)


Suggested change

.filter(p -> p.getSex().equals(Person.Sex.MAN)

.filter(p -> p.getSex() == Person.Sex.MAN

the same

liliia-ponomarenko · 2024-12-25T08:23:42Z

src/main/java/practice/StreamPractice.java

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


Suggested change

.filter(person -> ((person.getSex() == Person.Sex.MAN

&& person.getAge() > fromAge

&& person.getAge() <= maleToAge)

|| (person.getSex().equals(Person.Sex.WOMAN)

&& person.getAge() >= fromAge

&& person.getAge() <= femaleToAge)))

Predicate<Person> malePredicate = p -> p.getSex() == Person.Sex.MAN

&& p.getAge() >= fromAge && p.getAge() <= maleToAge;

Predicate<Person> femalePredicate = p -> p.getSex() == Person.Sex.WOMAN

&& p.getAge() >= fromAge && p.getAge() <= femaleToAge;

return peopleList.stream()

.filter(malePredicate.or(femalePredicate))

You can do it in this way

liliia-ponomarenko · 2024-12-25T08:25:16Z

src/main/java/practice/StreamPractice.java

@@ -59,7 +86,13 @@ 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) {
-        return Collections.emptyList();
+        return peopleList.stream()
+                .filter(person -> person.getSex().equals(Person.Sex.WOMAN)


Suggested change

.filter(person -> person.getSex().equals(Person.Sex.WOMAN)

.filter(person -> person.getSex() == Person.Sex.WOMAN

the same

valid solution

aadba1e

mateacademy-ai-mentor suggested changes Dec 21, 2024

View reviewed changes

implemented suggestions

28d2904

ihor-sydorenko requested a review from mateacademy-ai-mentor December 22, 2024 21:11

mateacademy-ai-mentor approved these changes Dec 22, 2024

View reviewed changes

liliia-ponomarenko suggested changes Dec 23, 2024

View reviewed changes

ihor-sydorenko added 2 commits December 24, 2024 22:49

improved solution

7992649

improved improved solution

6174f33

ihor-sydorenko requested a review from liliia-ponomarenko December 24, 2024 20:57

liliia-ponomarenko approved these changes Dec 25, 2024

View reviewed changes

implemented minor suggestions

7f8f338

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

f-hw-jv-stream-github-practice #1208

f-hw-jv-stream-github-practice #1208

ihor-sydorenko commented Dec 21, 2024

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor Dec 21, 2024

mateacademy-ai-mentor Dec 21, 2024

mateacademy-ai-mentor Dec 21, 2024

mateacademy-ai-mentor Dec 21, 2024

mateacademy-ai-mentor Dec 21, 2024

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor Dec 22, 2024

mateacademy-ai-mentor Dec 22, 2024

mateacademy-ai-mentor Dec 22, 2024

liliia-ponomarenko left a comment

liliia-ponomarenko Dec 23, 2024

liliia-ponomarenko Dec 23, 2024

liliia-ponomarenko Dec 23, 2024

liliia-ponomarenko Dec 23, 2024

liliia-ponomarenko Dec 23, 2024

liliia-ponomarenko Dec 23, 2024

liliia-ponomarenko Dec 23, 2024

ihor-sydorenko Dec 24, 2024

liliia-ponomarenko Dec 23, 2024

liliia-ponomarenko Dec 23, 2024

liliia-ponomarenko left a comment

liliia-ponomarenko Dec 25, 2024

liliia-ponomarenko Dec 25, 2024

liliia-ponomarenko Dec 25, 2024

		} catch (NumberFormatException e) {
		System.out.println("String does not contain a parsable integer");

		&& person.getAge() > fromAge
		&& person.getAge() <= maleToAge)

	public static int getLivePeriodInUkraine(String periodsInUkr) {
	private int getLivePeriodInUkraine(String periodsInUkr) {

	.filter(person -> ((person.getSex().equals(Person.Sex.MAN)
	.filter(person -> ((person.getSex() == Person.Sex.MAN

	.filter(p -> p.getSex().equals(Person.Sex.MAN)
	.filter(p -> p.getSex() == Person.Sex.MAN

	.filter(person -> person.getSex().equals(Person.Sex.WOMAN)
	.filter(person -> person.getSex() == Person.Sex.WOMAN

f-hw-jv-stream-github-practice #1208

Are you sure you want to change the base?

f-hw-jv-stream-github-practice #1208

Conversation

ihor-sydorenko commented Dec 21, 2024

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

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

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