[사다리타기] 1단계 - 자바8 스트림, 람다, Optional #1363
Conversation
Google Code Style 적용
Lambda 적용 - Car Test 익명 클래스를 람다로 전환 - sumAll 메소드 람다를 활용해 중복 제거
Lambda 적용 - countWords() Stream 적용 - sumOverThreeAndDouble() 메서드 구현 - printLongestWordTop100 구현
Optional 적용 - User ageIsInRange2() 메서드 구현 - Users getUser() 메서드 Optional 활용 구현 - Expression of() 메서드 Optional 활용 구현 - ComputerStore getVersionOptional() 메서드 Optional 활용 구현
| new Thread(new Runnable() { | ||
| @Override | ||
| public void run() { | ||
| System.out.println("Hello from thread"); | ||
| } | ||
| }).start(); |
| public static int sumAll(List<Integer> numbers, Predicate<Integer> predicate) { | ||
| return numbers.stream() | ||
| .filter(predicate) | ||
| .mapToInt(Integer::intValue) | ||
| .sum(); | ||
| } |
There was a problem hiding this comment.
이게 이름이 적절할까요? 무조건 전부를 더하는게 아닌 필터 조건이 등러가는데 말이죠. 요구조건중에 이 필터 역할을 할 수 있는 Condition 인터페이스를 정의하라고했는데, 안되있는 것 같네요 ㅠ
There was a problem hiding this comment.
넵 메서드명과 자바에서 제공하는 Predicate가 아닌 별도로 Condition인터페이스로 개선하도록 해보겠습니다!
| public static int sumAllEven(List<Integer> numbers) { | ||
| int total = 0; | ||
| for (int number : numbers) { | ||
| if (number % 2 == 0) { | ||
| total += number; | ||
| } |
There was a problem hiding this comment.
반복문안에 분기문이 있네요. 객체지향 생활체조원칙을 보면 indent가 2 이상이 되지 않도록 하라고 했죠 😢
또한 람다식으로 변경하라고 한 것이니 이렇게 for문을 사용하기보다는 람다식을 사용해보는게 어떨까요?
There was a problem hiding this comment.
아 넵 이 부분은 위에 Predicate를 사용한 sumAll을 구현하는 것으로 생각하여 일부러 남겨 두었었는데, 다음 미션에 람다로 수정해 놓을게요!
| public static int sumAllOverThree(List<Integer> numbers) { | ||
| int total = 0; | ||
| for (int number : numbers) { | ||
| if (number > 3) { | ||
| total += number; | ||
| } |
There was a problem hiding this comment.
여기도 직접 for문이 있네요 미션 요구사항에 람다식을 사용하라고 있었는데 말이죠 😢
리팩토링 해보시겠어요? 짝수더하기와 3이상 더하기 둘 다 덧셈이라는 부분과 조건이 있다는 공통점이 있으니, 일반화하여 메서드를 호출할 수 있도록 하면 좋을 것 같은데 마침 그 역할의 로직이 위에 있네요 ㅎ
There was a problem hiding this comment.
이 부분 또한 Predicate를 사용한 sumAll을 구현하는 것으로 생각하여 일부러 남겨 두었었는데, 다음 미션에 람다로 수정해 놓을게요!
| } | ||
| return isInRange; | ||
| } | ||
| public static boolean ageIsInRange1(User user) { |
There was a problem hiding this comment.
Optional 요구사항에 user 인수를 Optional을 사용하하는 조건이 있었습니다. 요구사항을 적용해보시겠어요?
There was a problem hiding this comment.
미션 요구사항에 아래와 같이 설명되어있어서, ageIsInRange2 메서드에서 구현하였는데 ageIsInRange1 메서드도 개선해야하는 걸까요..?
"같은 기능을 Optional을 활용해 ageIsInRange2() 메소드에 구현한다. 메소드 인자로 받은 User를 Optional로 생성하면 stream의 map, filter와 같은 메소드를 사용하는 것이 가능하다."
| assertThat(ageIsInRange1(new User("crong", 35))).isTrue(); | ||
| assertThat(ageIsInRange1(new User("crong", 48))).isFalse(); | ||
| assertThat(ageIsInRange1(new User("crong", null))).isFalse(); | ||
| assertThat(ageIsInRange1(new User("crong", 29))).isFalse(); | ||
| assertThat(ageIsInRange1(null)).isFalse(); |
There was a problem hiding this comment.
이 테스트 코드는 제공되는 테스트 코드여서 별도로 개선은 하지 않았습니다!
| @Test | ||
| public void getUser() { | ||
| Users users = new Users(); | ||
| assertThat(users.getUser("crong")).isEqualTo(new User("crong", 35)); | ||
| } |
There was a problem hiding this comment.
이게 무슨테스트인지에 대한 설명이 있으면 좋지 않을까요?
테스트가 1개~10개단위일때는 직접 로직을 보고 판단해도 좋겠지만, 프로젝트가 커져서
테스트가 1000개 이상이 된다면, 하나하나 모든 메서드의 로직을 보고 판단하기는 어려울 것 같아요.
There was a problem hiding this comment.
아 이 테스트 코드는 제공되는 테스트 코드여서 별도로 개선은 하지 않았습니다!
안녕하세요, 새로운 리뷰어님!
[사다리타기] 1단계 - 자바8 스트림, 람다, Optional 완료하여 PR드립니다!
리뷰 잘 부탁드릴게요!! :)
감사합니다!