-
Notifications
You must be signed in to change notification settings - Fork 1
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
1️⃣ 자바8 스트림, 람다, Optional #2
base: this-is-spear
Are you sure you want to change the base?
Conversation
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.
고생하셨습니다 :) 리뷰하면서 배울 점이 많네요!
return sumAll(numbers, Lambda::getOverThree); | ||
} | ||
|
||
private static int sumAll(List<Integer> numbers, Condition condition) { |
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.
condition을 직접 주지 않고 메소드로 분리해 사용하셨군요! 분리하니 확장에 유리한 코드가 된 것 같습니다. 저도 참고하겠습니다 :)
.reduce(Integer::sum).orElse(0); | ||
} | ||
|
||
private static void print(String Hello_from_thread) { |
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.
print 메소드를 따로 구현한 이유가 궁금합니다. 가독성을 위한 건가요?!
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.
이 부분은 제가 추가하지 않고 기존 코드에 존재한 코드이다 보니 리팩토링을 따로 진행하지 않았던 것 같아요..!! 사용하지 않는다면 삭제하겠습니다!
isInRange = true; | ||
} | ||
return isInRange; | ||
} | ||
|
||
public static boolean ageIsInRange2(User user) { | ||
return false; | ||
return user != null && user.getAge() != null && (user.getAge() >= 30 && user.getAge() <= 45); |
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.
Optional을 사용하여 구현하는 부분이었던 것 같은데, 사용되지 않았네요.
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.
그렇군요..!! 수정하겠습니다.
|
||
Map<String, Integer> wordMap = new HashMap<>(); | ||
|
||
for (String word : words) { |
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.
중복 제거를 위한 코드인가요? :) map으로 저장하신 의도가 궁금합니다!
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.
맞습니다. 중복 제거를 위한 코드입니다. 👍
return Arrays.stream(values()) | ||
.filter(expression1 -> matchExpression(expression1, expression)) | ||
.findFirst() | ||
.orElseThrow( |
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.
줄 구분을 하니 깔끔하게 읽히네요
저도 줄바꿈하는 습관을 들여야겠습니다 👍🏼
} | ||
return DEFAULT_USER; | ||
Optional<User> getUser(String name) { | ||
return users.stream().filter(user -> user.matchName(name)).findFirst(); |
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.
orElse(DEFAULT_USER)가 이부분에 구현되어야 하지 않나요?!
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.
Optional로 값을 리턴해 NUll 값을 허용했는데, Users에서 Optional을 넘기는 방식이 옳은건지 궁금해지네요! DEFAULT_USER
로 리턴하는 방식이 맞는 것 같습니다.
No description provided.