Created first version of the program. #912

ponomvrenko · 2023-10-26T13:00:14Z

No description provided.

fedorovychh

You`ve done well-written solution! I commented a few points that will do your code a little more optimized, pay attention to it please. And do not forget magical numbers and constant fields.

fedorovychh · 2023-10-26T18:31:23Z

src/main/java/practice/CandidateValidator.java

+    private static final int TO_YEAR_INDEX = 1;
+
+    @Override
+    public boolean test(Candidate c) {


I would recommend to change Candidate variable name to something more informational

fedorovychh · 2023-10-26T18:34:32Z

src/main/java/practice/StreamPractice.java

+        if (numbers.size() == 0) {
+            throw new RuntimeException("Can't get min value from list:" + numbers);
+        }


It will mcuh better to delete this if/else construction and use one of Optional methods

In addition, you can make exception message constant

Great suggestion, it removes useless if-else with orElseThrow() method

fedorovychh · 2023-10-26T18:37:11Z

src/main/java/practice/StreamPractice.java

+            throw new RuntimeException("Can't get min value from list:" + numbers);
+        }
+        return numbers.stream()
+                .map(s -> List.of(s.replace(" ", "").split(",")))


You can make regex constant. And why do you need s.replace here? It seems we can avoid using replace method

Haha, honestly idk, I just imagine that we have spaces around our numbers, and we have to handle this situation. Thanks!

fedorovychh · 2023-10-26T18:39:51Z

src/main/java/practice/StreamPractice.java

+        }
+        return numbers.stream()
+                .map(s -> List.of(s.replace(" ", "").split(",")))
+                .flatMapToInt(l -> l.stream().mapToInt(Integer::parseInt))


Yes, it works properly. But it can be simplified by using just flatmap. You don`t need to change it, but think about it.

fedorovychh · 2023-10-26T18:41:00Z

src/main/java/practice/StreamPractice.java

+                .filter(p -> fromAge <= p.getAge() && p.getAge() <= toAge
+                        && p.getSex() == Person.Sex.MAN)


It`ll better to compare getSex and Sex.VALUE by equals

It is permissible to use the == operator in place of the equals method when comparing two object references if it is known that at least one of them refers to an enum constant.

https://stackoverflow.com/questions/1750435/comparing-java-enum-members-or-equals/1750453#1750453

fedorovychh · 2023-10-26T18:44:07Z

src/main/java/practice/StreamPractice.java

+                .map(Person::getCats)
+                .flatMap(Collection::stream)
+                .map(Cat::getName)
+                .collect(Collectors.toList());


Maybe you can simplify it by using .toList() instead of .collect(Collectors.toList())

1. Renamed parameter name in CandidateValidator test(); 2. Optimized findMinEvenNumber(): a) added constants for RuntimeException and REGEX; b) Removed useless replace(); c) optimized stream with flatMap() and orElseThrow(); 3. Optimized selectNenByAge() method;

ponomvrenko · 2023-10-27T01:46:08Z

@proxychang thanks for your review! I just change my code with your suggestions, and it becomes more elegant <3

mnyshenko

A few point to consider

mnyshenko · 2023-10-27T10:35:54Z

src/main/java/practice/StreamPractice.java

+                .map(s -> List.of(s.split(REGEX)))
+                .flatMap(Collection::stream)
+                .mapToInt(Integer::parseInt)
+                .filter(n -> n % 2 == 0)
+                .min()


It's better to give more readable names for your parameters in lambdas. One-letter names are usually used only in for-loop declarations

mnyshenko · 2023-10-27T10:51:40Z

src/main/java/practice/StreamPractice.java

    public Double getOddNumsAverage(List<Integer> numbers) {
-        return 0D;
+        return IntStream.range(0, numbers.size())
+                .map(i -> i % 2 != 0 ? numbers.get(i) - 1 : numbers.get(i))
+                .boxed()
+                .filter(n -> n % 2 != 0)
+                .mapToDouble(n -> n)
+                .average()
+                .getAsDouble();
    }


A few points here:

Naming, one-letter naming are acceptable only when creating for-loop

a few redundant actions here; please take a look at given implementation

don't repeat yourself and extract identical parts inside methods/variables for further reuse: n -> n % 2 != 0

public static Double getOddNumsAverage(List<Integer> numbers) { return IntStream.range(0, numbers.size()) .map(index -> isOddNumber(index) ? numbers.get(index) - 1 : numbers.get(index)) .filter(NextBiggerNumber::isOddNumber) .average() .getAsDouble(); } private static boolean isOddNumber(int number) { return number % 2 != 0; }

mnyshenko · 2023-10-27T10:52:12Z

src/main/java/practice/StreamPractice.java

+                .filter(p -> fromAge <= p.getAge() && p.getAge() <= toAge
+                        && p.getSex() == Person.Sex.MAN)


One-letter naming

mnyshenko · 2023-10-27T10:53:13Z

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))


p.getAge() used twice so can be extracted to a separate variable, same with sex

…d isOddNumber, created predicate for a big condition in getWorkablePeople().

# Conflicts: # src/main/java/practice/StreamPractice.java

…od isOddNumber(), created predicate for a big condition in getWorkablePeople().

bhdnchui

Good job, but a few minor points need to consider.

src/main/java/practice/CandidateValidator.java

bhdnchui · 2023-10-27T15:15:13Z

src/main/java/practice/StreamPractice.java

+        return IntStream.range(0, numbers.size())
+                .map(index -> isOddNumber(index) ? numbers.get(index) - 1 : numbers.get(index))
+                .filter(StreamPractice::isOddNumber)
+                .average()


Check if there is an average number. If there is not, throw NoSuchElementException according to the task requirements.

@bhdnchui I have there getAsDouble() method that includes
if (!isPresent) { throw new NoSuchElementException("No value present"); }
Is my solution here incorrect?

src/main/java/practice/StreamPractice.java

bhdnchui

Good job!

Created first version of the program.

e06901e

fedorovychh suggested changes Oct 26, 2023

View reviewed changes

Student review changes:

031e29e

1. Renamed parameter name in CandidateValidator test(); 2. Optimized findMinEvenNumber(): a) added constants for RuntimeException and REGEX; b) Removed useless replace(); c) optimized stream with flatMap() and orElseThrow(); 3. Optimized selectNenByAge() method;

mnyshenko suggested changes Oct 27, 2023

View reviewed changes

ponomvrenko added 3 commits October 27, 2023 17:40

Created constants, optimized getOddNumsAverage(), created a new metho…

7ff3a0e

…d isOddNumber, created predicate for a big condition in getWorkablePeople().

Merge remote-tracking branch 'origin/main'

b6ab569

# Conflicts: # src/main/java/practice/StreamPractice.java

Optimized findMinEvenNumber & getOddNumsAverage(), created a new meth…

26fb41b

…od isOddNumber(), created predicate for a big condition in getWorkablePeople().

bhdnchui suggested changes Oct 27, 2023

View reviewed changes

Minor updates

656051a

ponomvrenko requested a review from bhdnchui October 27, 2023 15:24

bhdnchui approved these changes Oct 27, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Created first version of the program. #912

Created first version of the program. #912

ponomvrenko commented Oct 26, 2023

fedorovychh left a comment •

edited

Loading

fedorovychh Oct 26, 2023

fedorovychh Oct 26, 2023

fedorovychh Oct 26, 2023

ponomvrenko Oct 27, 2023

fedorovychh Oct 26, 2023

ponomvrenko Oct 27, 2023

fedorovychh Oct 26, 2023

fedorovychh Oct 26, 2023

ponomvrenko Oct 27, 2023

fedorovychh Oct 26, 2023

ponomvrenko commented Oct 27, 2023

mnyshenko left a comment

mnyshenko Oct 27, 2023

mnyshenko Oct 27, 2023

mnyshenko Oct 27, 2023

mnyshenko Oct 27, 2023

bhdnchui left a comment

bhdnchui Oct 27, 2023

ponomvrenko Oct 27, 2023

bhdnchui left a comment

		.filter(p -> fromAge <= p.getAge() && p.getAge() <= toAge
		&& p.getSex() == Person.Sex.MAN)

Created first version of the program. #912

Are you sure you want to change the base?

Created first version of the program. #912

Conversation

ponomvrenko commented Oct 26, 2023

fedorovychh left a comment • edited Loading

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

ponomvrenko commented Oct 27, 2023

mnyshenko 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

bhdnchui left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bhdnchui left a comment

Choose a reason for hiding this comment

fedorovychh left a comment •

edited

Loading