-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Implemented all methods #916
base: main
Are you sure you want to change the base?
Conversation
&& candidate.getAge() >= 35 | ||
&& candidate.getNationality().equals("Ukrainian")) { | ||
int startYear = Integer.parseInt( | ||
candidate.getPeriodsInUkr().split("-")[0]); | ||
int endYear = Integer.parseInt( | ||
candidate.getPeriodsInUkr().split("-")[1]); | ||
return endYear - startYear >= 10; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don`t use magic numbers
.filter(e -> e % 2 == 0) | ||
.sorted() | ||
.findFirst() | ||
.orElseThrow(() -> new RuntimeException("Can't get min value from list: " + numbers)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be constant
}) | ||
.filter(number -> number % 2 != 0) | ||
.average() | ||
.orElseThrow(() -> new NoSuchElementException("Empty List provided")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be constant
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job!
A few comments need to be process.
int startYear = Integer.parseInt( | ||
candidate.getPeriodsInUkr().split("-")[0]); | ||
int endYear = Integer.parseInt( | ||
candidate.getPeriodsInUkr().split("-")[1]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make one variable for data that indicates how long a candidate is in country.
Also, it would be great to make constants for index numbers. Constant names will descript what item you take.
Double .split("-")
call. Make constant for "-"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, refactored a little, but I can make it only in one barely readable line.
Could you please make a suggestion?
public int findMinEvenNumber(List<String> numbers) { | ||
return 0; | ||
return numbers.stream() | ||
.map(e -> e.split(",")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's not use one-letter names.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make constant for comma.
if (index % 2 != 0) { | ||
return numbers.get(index) - 1; | ||
} | ||
return numbers.get(index); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can reduce code by providing a ternary operator.
} | ||
return numbers.get(index); | ||
}) | ||
.filter(number -> number % 2 != 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets avoid duplication of code that checks odd/even number.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, need a suggestion here. Spent more than an hour on this
return Collections.emptyList(); | ||
return peopleList.stream() | ||
.filter(person -> person.getAge() >= fromAge && person.getAge() <= toAge) | ||
.filter(person -> person.getSex().name().equals("MAN")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use enum instead string literal.
public List<Person> selectMenByAge(List<Person> peopleList, int fromAge, int toAge) { | ||
return Collections.emptyList(); | ||
return peopleList.stream() | ||
.filter(person -> person.getAge() >= fromAge && person.getAge() <= toAge) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Double getAge method call.
return person.getAge() >= fromAge && person.getAge() <= maleToAge; | ||
} | ||
return person.getAge() >= fromAge && person.getAge() <= femaleToAge; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make a variable for name.
Predicate<Person> isWorkable = new Predicate<Person>() { | ||
@Override | ||
public boolean test(Person person) { | ||
if (person.getSex().name().equals("MAN")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
enum
.map(e -> e.split(",")) | ||
.flatMap(Arrays::stream) | ||
.map(Integer::parseInt) | ||
.filter(e -> e % 2 == 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One-letter name
public List<Person> getWorkablePeople(int fromAge, int femaleToAge, | ||
int maleToAge, List<Person> peopleList) { | ||
return Collections.emptyList(); | ||
int maleToAge, List<Person> peopleList) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Format that line of code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this 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.
.filter(number -> number % 2 == 0) | ||
.sorted() | ||
.findFirst() | ||
.orElseThrow(() -> new RuntimeException(NO_MIN_VALUE_EXCEPTION_MESSAGE + numbers)); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.filter(number -> number % 2 == 0) | |
.sorted() | |
.findFirst() | |
.orElseThrow(() -> new RuntimeException(NO_MIN_VALUE_EXCEPTION_MESSAGE + numbers)); | |
} | |
.filter(number -> !isOddNumber(number)) | |
.sorted() | |
.findFirst() | |
.orElseThrow(() -> new RuntimeException(NO_MIN_VALUE_EXCEPTION_MESSAGE + numbers)); | |
} | |
private static boolean isOddNumber(int number) { | |
return number % 2 != 0; | |
} |
int personAge = person.getAge(); | ||
; | ||
return personAge >= fromAge | ||
&& personAge <= toAge | ||
&& person.getSex().equals(Sex.MAN); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
int personAge = person.getAge(); | |
; | |
return personAge >= fromAge | |
&& personAge <= toAge | |
&& person.getSex().equals(Sex.MAN); | |
int personAge = person.getAge(); | |
return personAge >= fromAge | |
&& personAge <= toAge | |
&& Sex.MAN.equals(person.getSex()); |
It will avoid NPE.
public List<Person> getWorkablePeople(int fromAge, int femaleToAge, | ||
int maleToAge, List<Person> peopleList) { | ||
return Collections.emptyList(); | ||
int maleToAge, List<Person> peopleList) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
int maleToAge, List<Person> peopleList) { | ||
Predicate<Person> isWorkable = person -> { | ||
int personAge = person.getAge(); | ||
if (person.getSex().equals(Sex.MAN)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (person.getSex().equals(Sex.MAN)) { | |
if (Sex.MAN.equals(person.getSex())) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job!
No description provided.