Solve tasks from Readme file #929

DEz-1996 · 2023-10-30T00:00:11Z

No description provided.

MinoDentori

Good job! There are some points to check, but they have high occurrence rates.

MinoDentori · 2023-10-30T21:59:54Z

src/main/java/practice/CandidateValidator.java

+    public boolean test(Candidate candidate) {
+        return candidate.isAllowedToVote()
+                && candidate.getAge() >= MIN_VALID_AGE
+                && candidate.getNationality().equals(VALID_NATIONALITY)


We should build construction in this way to avoid NPE.
VALID_NATIONALITY.equals(candidate.getNationality())

MinoDentori · 2023-10-30T22:00:34Z

src/main/java/practice/CandidateValidator.java

+                liveInUkraine.substring(0,liveInUkraine.indexOf(DIVIDER_IN_LIVE_PERIOD)));
+        int upToYear = Integer.parseInt(
+                liveInUkraine.substring(liveInUkraine.indexOf(DIVIDER_IN_LIVE_PERIOD) + 1));


We can make constants for the indexes.

MinoDentori · 2023-10-30T22:02:37Z

src/main/java/practice/StreamPractice.java

+        Supplier<RuntimeException> exceptionSupplier = () ->
+                new RuntimeException("Can't get min value from list: " + numbers);
+        return numbers.stream()
+                .map(s -> s.split(NUMBERS_DIVIDER))


We should use full-named names for the variables. (we know, that it depends on the project, but for this task, it is possible to be rejected in that case, so comment)

MinoDentori · 2023-10-30T22:03:16Z

src/main/java/practice/StreamPractice.java

+                .map(i -> (i % 2 != 0) ? numbers.get(i) - 1 : numbers.get(i))
+                .filter(n -> n % 2 == 1)
+                .mapToInt(n -> n)


We should use full-named names for the variables.

MinoDentori · 2023-10-30T22:04:25Z

src/main/java/practice/StreamPractice.java

    public List<Person> selectMenByAge(List<Person> peopleList, int fromAge, int toAge) {
-        return Collections.emptyList();
+        Predicate<Person> predicate = person ->
+                person.getSex() == Person.Sex.MAN


We should build construction in this way to avoid NPE and MB use equals.
Person.Sex.MAN == person.getSex()

We compare two enums with equal references, in this case we can use ==

MinoDentori · 2023-10-30T22:08:15Z

src/main/java/practice/StreamPractice.java

+                (person.getSex() == Person.Sex.MAN && person.getAge() <= maleToAge)
+                || (person.getSex() == Person.Sex.WOMAN && person.getAge() <= femaleToAge);


We should build construction in this way to avoid NPE and MB use equals.
Can we create variables to store person.getSex() and person.getAge() values?

We compare two enums with equal references, in this case we can use ==
I think create variables is redundant in this place

MinoDentori · 2023-10-30T22:08:32Z

src/main/java/practice/StreamPractice.java

+        return peopleList.stream()
+                .filter(p -> p.getAge() >= fromAge)
+                .filter(predicate)
+                .collect(Collectors.toList());


Use .toList if it is possible.

MinoDentori · 2023-10-30T22:08:39Z

src/main/java/practice/StreamPractice.java

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


Use .toList if it is possible.

MinoDentori · 2023-10-30T22:09:07Z

src/main/java/practice/StreamPractice.java

    public List<String> getCatsNames(List<Person> peopleList, int femaleAge) {
-        return Collections.emptyList();
+        return peopleList.stream()
+                .filter(p -> p.getSex() == Person.Sex.WOMAN && p.getAge() >= femaleAge)


We should build construction in this way to avoid NPE and MB use equals.
Person.Sex.MAN == person.getSex()

(null == enum.value) return boolean false
(null.eauals(enum.value)) throw NPE

MinoDentori · 2023-10-30T22:09:26Z

src/main/java/practice/StreamPractice.java

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


Use .toList if it is possible.

d1sam

Good job, but some changes should be done.

d1sam · 2023-10-31T11:12:38Z

src/main/java/practice/CandidateValidator.java

+        int fromYear = Integer.parseInt(
+                liveInUkraine.substring(START_SUBSTRING_INDEX,
+                        liveInUkraine.indexOf(DIVIDER_IN_LIVE_PERIOD)));
+        int upToYear = Integer.parseInt(
+                liveInUkraine.substring(liveInUkraine.indexOf(DIVIDER_IN_LIVE_PERIOD)
+                        + SLIDER_TO_START_SUBSTRING_INDEX));


please better use .split(DIVIDER_IN_LIVE_PERIOD) to make code more readable.

d1sam · 2023-10-31T11:12:45Z

src/main/java/practice/CandidateValidator.java

+    private static final int START_SUBSTRING_INDEX = 0;
+    private static final int SLIDER_TO_START_SUBSTRING_INDEX = 1;
+    private static final String VALID_NATIONALITY = "Ukrainian";
+    private static final char DIVIDER_IN_LIVE_PERIOD = '-';


Suggested change

private static final char DIVIDER_IN_LIVE_PERIOD = '-';

private static final char DIVIDER_DASH = '-';

d1sam · 2023-10-31T11:13:13Z

src/main/java/practice/StreamPractice.java

+                .map(string -> string.split(NUMBERS_DIVIDER))
+                .flatMap(Arrays::stream)
+                .map(Integer::parseInt)
+                .filter(number -> number % 2 == 0)


extract i % 2 == 0 to a separate method isEven

d1sam · 2023-10-31T11:14:58Z

src/main/java/practice/StreamPractice.java

+        return numbers.stream()
+                .map(string -> string.split(NUMBERS_DIVIDER))
+                .flatMap(Arrays::stream)
+                .map(Integer::parseInt)


Suggested change

.map(Integer::parseInt)

.mapToInt(Integer::parseInt)

d1sam · 2023-10-31T11:15:33Z

src/main/java/practice/StreamPractice.java

+                .flatMap(Arrays::stream)
+                .map(Integer::parseInt)
+                .filter(number -> number % 2 == 0)
+                .min(Comparator.naturalOrder())


Suggested change

.min(Comparator.naturalOrder())

.min()

When you have IntStream Comparator.naturalOrder() is redundant

d1sam · 2023-10-31T11:17:38Z

src/main/java/practice/StreamPractice.java

+        return Stream.iterate(0, integer -> integer + 1).limit(numbers.size())
+                .peek(System.out::println)
+                .map(number -> (number % 2 != 0) ? numbers.get(number) - 1 : numbers.get(number))
+                .filter(number -> number % 2 == 1)


extract i % 2 == 0 to a separate method isEven

d1sam · 2023-10-31T11:18:03Z

src/main/java/practice/StreamPractice.java

    public Double getOddNumsAverage(List<Integer> numbers) {
-        return 0D;
+        return Stream.iterate(0, integer -> integer + 1).limit(numbers.size())
+                .peek(System.out::println)


Suggested change

.peek(System.out::println)

d1sam · 2023-10-31T11:21:57Z

src/main/java/practice/StreamPractice.java

    public Double getOddNumsAverage(List<Integer> numbers) {
-        return 0D;
+        return Stream.iterate(0, integer -> integer + 1).limit(numbers.size())


Suggested change

return Stream.iterate(0, integer -> integer + 1).limit(numbers.size())

return IntStream.range(0, numbers.size())

d1sam · 2023-10-31T11:22:08Z

src/main/java/practice/StreamPractice.java

+                .peek(System.out::println)
+                .map(number -> (number % 2 != 0) ? numbers.get(number) - 1 : numbers.get(number))
+                .filter(number -> number % 2 == 1)
+                .mapToInt(number -> number)


Suggested change

.mapToInt(number -> number)

d1sam · 2023-10-31T11:24:44Z

src/main/java/practice/StreamPractice.java

    public List<String> validateCandidates(List<Candidate> candidates) {
-        return Collections.emptyList();
+        CandidateValidator candidateValidator = new CandidateValidator();


please declare candidateValidator variable in the class-level access

mnyshenko

Approved, but pay attention to the comments

mnyshenko · 2023-10-31T13:05:17Z

src/main/java/practice/CandidateValidator.java

+        int fromYear = Integer.parseInt(liveInUkraine.split(DIVIDER_DASH)[FROM_YEAR_INDEX]);
+        int upToYear = Integer.parseInt(liveInUkraine.split(DIVIDER_DASH)[UP_TO_YEAR_INDEX]);


liveInUkraine.split(DIVIDER_DASH) can be extracted to a separate variable, so we can split only once

mnyshenko · 2023-10-31T13:06:48Z

src/main/java/practice/StreamPractice.java

    public Double getOddNumsAverage(List<Integer> numbers) {
-        return 0D;
+        return IntStream.range(0, numbers.size())
+                .map(number -> (!isEven(number)) ? numbers.get(number) - 1 : numbers.get(number))


seems to be redundant parentheses (!isEven(number))

mnyshenko · 2023-10-31T13:10:36Z

src/main/java/practice/StreamPractice.java

+                && person.getAge() >= fromAge
+                && person.getAge() <= toAge;


We should always extract identical expressions into variables

Thus, we avoid code duplication

With current implementation If tomorrow you'll need to have person.getAge() + 10 expression here, you'll need to make changes in two places instead of one

mnyshenko · 2023-10-31T13:11:11Z

src/main/java/practice/StreamPractice.java

+                (person.getSex() == Person.Sex.MAN && person.getAge() <= maleToAge)
+                || (person.getSex() == Person.Sex.WOMAN && person.getAge() <= femaleToAge);


person.getAge() and person.getSex() duplication

mnyshenko · 2023-10-31T13:12:21Z

src/main/java/practice/StreamPractice.java

    public List<Person> getWorkablePeople(int fromAge, int femaleToAge,
                                          int maleToAge, List<Person> peopleList) {
-        return Collections.emptyList();
+        Predicate<Person> predicate = person ->


predicate name is too generic, it's better to give some name that explains what this predicate does

Solve tasks from Readme file

8bca626

MinoDentori suggested changes Oct 30, 2023

View reviewed changes

DEz-1996 added 2 commits October 31, 2023 12:34

Temp commit before update

187ed3e

Some fixes with student's advices

38f81ba

d1sam suggested changes Oct 31, 2023

View reviewed changes

Fixes with mentor's advices

31ad4e0

mnyshenko approved these changes Oct 31, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Solve tasks from Readme file #929

Solve tasks from Readme file #929

DEz-1996 commented Oct 30, 2023

MinoDentori left a comment •

edited

Loading

MinoDentori Oct 30, 2023

MinoDentori Oct 30, 2023

MinoDentori Oct 30, 2023

MinoDentori Oct 30, 2023

MinoDentori Oct 30, 2023

DEz-1996 Oct 31, 2023

MinoDentori Oct 30, 2023

DEz-1996 Oct 31, 2023

MinoDentori Oct 30, 2023

MinoDentori Oct 30, 2023

MinoDentori Oct 30, 2023

DEz-1996 Oct 31, 2023

MinoDentori Oct 30, 2023

d1sam left a comment

d1sam Oct 31, 2023

d1sam Oct 31, 2023

d1sam Oct 31, 2023

d1sam Oct 31, 2023

d1sam Oct 31, 2023

d1sam Oct 31, 2023

d1sam Oct 31, 2023

d1sam Oct 31, 2023

d1sam Oct 31, 2023

d1sam Oct 31, 2023

d1sam Oct 31, 2023

mnyshenko left a comment

mnyshenko Oct 31, 2023

mnyshenko Oct 31, 2023

mnyshenko Oct 31, 2023

mnyshenko Oct 31, 2023

mnyshenko Oct 31, 2023

		(person.getSex() == Person.Sex.MAN && person.getAge() <= maleToAge)
		\|\| (person.getSex() == Person.Sex.WOMAN && person.getAge() <= femaleToAge);

	private static final char DIVIDER_IN_LIVE_PERIOD = '-';
	private static final char DIVIDER_DASH = '-';

	return Stream.iterate(0, integer -> integer + 1).limit(numbers.size())
	return IntStream.range(0, numbers.size())

		int fromYear = Integer.parseInt(liveInUkraine.split(DIVIDER_DASH)[FROM_YEAR_INDEX]);
		int upToYear = Integer.parseInt(liveInUkraine.split(DIVIDER_DASH)[UP_TO_YEAR_INDEX]);

Solve tasks from Readme file #929

Are you sure you want to change the base?

Solve tasks from Readme file #929

Conversation

DEz-1996 commented Oct 30, 2023

MinoDentori 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

d1sam 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

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

Choose a reason for hiding this comment

MinoDentori left a comment •

edited

Loading