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

stream-github-practice solution #924

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

Conversation

OlexVoit
Copy link

No description provided.

Copy link

@vadymhrnk vadymhrnk 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! Found only one thing that must be changed

Comment on lines 41 to 45
return peopleList.stream()
.filter(person -> person.getAge() >= fromAge
&& person.getAge() <= toAge
&& person.getSex() == Person.Sex.MAN)
.toList();

Choose a reason for hiding this comment

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

I think here you could split all statements in different filters to make it look less complicated

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

Choose a reason for hiding this comment

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

It's better when you create validator on class level, not when method calls

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 comments need to be processed.


@Override
public boolean test(Candidate candidate) {
String[] splitCandidate = candidate.getPeriodsInUkr().split(SEPARATION_MARK);

Choose a reason for hiding this comment

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

Find better name.

Comment on lines 16 to 20
int yearsInUkr = Arrays.stream(splitCandidate)
.map(String::trim)
.map(Integer::parseInt)
.reduce((a, b) -> b - a)
.orElse(0);

Choose a reason for hiding this comment

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

Can we make it with substruction?

.orElse(0);
return candidate.getAge() >= MIN_AGE
&& candidate.isAllowedToVote()
&& candidate.getNationality().equals(NATIONALITY)

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(NATIONALITY)
&& NATIONALITY.equals(candidate.getNationality())

public int findMinEvenNumber(List<String> numbers) {
return 0;
return numbers.stream()
.map(split -> split.split(","))

Choose a reason for hiding this comment

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

Make constant for comma.

Comment on lines 24 to 25
.sorted()
.findFirst()

Choose a reason for hiding this comment

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

Suggested change
.sorted()
.findFirst()
.min()

Comment on lines 30 to 34
List<Integer> filterListNumbers = IntStream.range(0, numbers.size())
.mapToObj(i -> i % 2 == 1 ? numbers.get(i) - 1 : numbers.get(i))
.toList();
return filterListNumbers.stream()
.filter(num -> num % 2 == 1)

Choose a reason for hiding this comment

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

Let's make it with one stream.
Dont use one-letter names.
Avoid duplication of code that checks even or odd is a number.

Comment on lines 41 to 43
Predicate<Person> predicate = person -> person.getAge() >= fromAge
&& person.getAge() <= toAge
&& person.getSex() == Person.Sex.MAN;

Choose a reason for hiding this comment

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

Make variables to avoid double getAge() method call
Pay attention to what is a better way to compare Enum values: equals() vs == ?

Comment on lines 52 to 58
if (person.getAge() >= fromAge) {
if (person.getSex() == Person.Sex.WOMAN && person.getAge() <= femaleToAge) {
return true;
}
return person.getSex() == Person.Sex.MAN && person.getAge() <= maleToAge;
}
return false;

Choose a reason for hiding this comment

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

Make variables to avoid double getAge() and getSex() methods calls
Pay attention to what is a better way to compare Enum values: equals() vs == ?

return Collections.emptyList();

return peopleList.stream()
.filter(women -> women.getSex() == Person.Sex.WOMAN && women.getAge() >= femaleAge)

Choose a reason for hiding this comment

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

.filter(new CandidateValidator())
.map(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.

Suggested change
.collect(Collectors.toList());
.toList();

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