Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added method implementations and created a CandidateValidator #921

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

DevIgork
Copy link

No description provided.

Comment on lines 9 to 15
String[] ages = candidate.getPeriodsInUkr().split("-");
int ageFrom = Integer.parseInt(ages[0]);
int ageTo = Integer.parseInt(ages[1]);
return candidate.getAge() >= 35
&& candidate.isAllowedToVote()
&& candidate.getNationality().equals("Ukrainian")
&& ageTo - ageFrom >= 10;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi mate i'm sure that we can move all magic numbers and values to constants.

@@ -14,7 +17,14 @@ 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().flatMap(num -> {
String[] numString = num.split(",");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably here we can move comma to constant too

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make constant

@@ -14,7 +17,14 @@ public class StreamPractice {
* "Can't get min value from list: < Here is our input 'numbers' >"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove all task descriptions from your code

&& pepl.getAge() <= maleToAge
: pepl.getAge() >= fromAge
&& pepl.getAge() <= femaleToAge
).collect(Collectors.toList());
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove

.flatMap(pepl -> pepl.getCats()
.stream()
.map(cat -> cat.getName()))
.collect(Collectors.toList());
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and here

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

/**

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here too

.mapToInt(n -> n)
.average()
.orElseThrow(NoSuchElementException::new);

}

/**

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and here

.filter(n -> n % 2 == 0);
}).min(Integer::compareTo)
.orElseThrow(() -> new RuntimeException("Can't get min value from list: "
+ numbers));
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

plus here

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I forgot about it, thank you.

Copy link

@bhdnchui bhdnchui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job, but a few points need to be processed,

public class CandidateValidator implements Predicate<Candidate> {
@Override
public boolean test(Candidate candidate) {
String[] ages = candidate.getPeriodsInUkr().split("-");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's find better name
Make constant for "-"

Comment on lines 10 to 11
int ageFrom = Integer.parseInt(ages[0]);
int ageTo = Integer.parseInt(ages[1]);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's create constants for indexes.

Comment on lines 12 to 15
return candidate.getAge() >= 35
&& candidate.isAllowedToVote()
&& candidate.getNationality().equals("Ukrainian")
&& ageTo - ageFrom >= 10;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's create constansts for ages and nationality.

int ageTo = Integer.parseInt(ages[1]);
return candidate.getAge() >= 35
&& candidate.isAllowedToVote()
&& candidate.getNationality().equals("Ukrainian")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
&& candidate.getNationality().equals("Ukrainian")
&& UKRAINIAN.equals(candidate.getNationality())

@@ -14,7 +17,14 @@ 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().flatMap(num -> {
String[] numString = num.split(",");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make constant

Comment on lines 91 to 93
.flatMap(pepl -> pepl.getCats()
.stream()
.map(cat -> cat.getName()))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's avoid stream in stream

.flatMap(pepl -> pepl.getCats()
.stream()
.map(cat -> cat.getName()))
.collect(Collectors.toList());

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same

CandidateValidator candidateValidator = new CandidateValidator();
return candidates
.stream()
.filter(candidate -> candidateValidator.test(candidate))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.filter(candidate -> candidateValidator.test(candidate))
.filter(candidateValidator)

return candidates
.stream()
.filter(candidate -> candidateValidator.test(candidate))
.map(candidate -> candidate.getName())

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.map(candidate -> candidate.getName())
.map(Candidate::getName)

.filter(candidate -> candidateValidator.test(candidate))
.map(candidate -> candidate.getName())
.sorted()
.collect(Collectors.toList());

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same

Copy link

@bhdnchui bhdnchui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants