Step1 스트림, 람다, Optional
평소에 자바8 문법을 사용하고 있어서 이번 Step은 별다른 어려움이 없었다.
.filter(age -> age >= 30 && age <= 45)
age 조건에 맞게 필터를 거는 로직이 있었다.
아래와 같은 피드백을 받았다.
.filter(User::허용검증)💡검증에대한 의미있는 이름을 지어보고, 객체에게 물어볼수도 있겠네요! 😄
다른 리뷰어분들께도 같은 피드백을 받은 적이 있다.
단순한 조건이여도 메소드로 추출하여 의미 있는 이름을 짓는게 코드 가독성 측면에서 좋다.
Optional<User> optionalUser = Optional.ofNullable(user);
return optionalUser
.map(User::getAge)
.filter(age -> age >= 30 && age <= 45)
.isPresent();
Optional에서 Map 함수를 사용하면 map 메소드 내의 결과값이 null이면 빈 Optional을 반환한다.
null 일 경우 예외를 던지고 싶으면 메소드 체이닝 마지막에 .orElseThrow()를 적용하면 된다.
Optional을 많이 사용했음에도 불구하고 Map 메소드를 사용할 생각을 하지 못했는데 이러한 방법도 알게 되어 좋았다.
Step2 사다리(생성)
사다리 생성을 처음 봤을때 도저히 감이 잡히지 않았다. 자동차 경주와 로또 미션과는 달리 화면에 사다리를 그려야 되는 요구사항이 있어서 그런 것 같다. 아니면 알고리즘 실력이 떨어져서 그런걸지도..?
※ TMI로 예전에는 알고리즘이 실무와 관련 없다고 생각했는데, 지금은 아니다. DFS, BFS, DP 와 같은 알고리즘 기술은 실무에서 많이 사용하진 않지만 알고리즘 문제를 풀기 위한 사고력은 실무와 연관이 깊은게 아닐까 한다.
사다리 미션은 감이 잡히질 않아서 다른 사람이 구현한 것을 참고하면서 개발했다.
public class ExceedNameLengthException extends RuntimeException {
private static final String MESSAGE = "참여자의 이름은 5자 이하로 입력하세요.";
예외처리에 대한 메시지를 요구사항에 맞춰서 작성하였다.
아래와 같은 피드백을 받았다.
💡 이름에 대한 조건이 변경되면 5자 에 대한 불일치가 발생할 수 있겠네요 😂
잘 이해가 가질 않아서 다시 여쭤보았다.
💡 안녕하세요!! 해당 내용이 잘 이해가 안가서요..
만약에 이름에 대한 조건이 5자 -> 10자로 변경 되면"참여자의 이름은 5자 이하로 입력하세요." -> "참여자의 이름은 10자 이하로 입력하세요."출력할 메시지를 변경하지 않으면 불일치가 발생한다.
따라서 요구사항에 변경에 대비하여 출력할 메시지를"참여자의 이름 길이가 초과되었습니다"이런식으로 변경하는게 좋다 라는 말씀이실까요?
💡 맞아요😀
저는 그러한 작업을 일반화하여 표기하기라고 해요.
아니면 커스텀 예외의 장점을 살려 파라미터를 전달할수도있겠죠 ㅎ
리뷰어님의 입장과는 다른 생각을 가지고 있다.
정책과 관련된 메시지를 남겨야 예외가 발생했을때 디버깅 하기가 쉽다고 생각한다.
일반화해서 표기 하면 정책이 무엇인지 찾아봐야 하는 비용이 든다.
물론 리뷰어님 말씀대로 정책이 수정되면 메시지는 변경해야 한다.
개인적으론 디버깅하기 편한게 이점이 더 크다고 생각한다.
Participants participatns = new Participants(persons);
1명 이상의 참가자 이름을 입력 받을 수 있어 Participants라는 클래스 명으로 참가자명을 관리하고 있었다.
아래와 같은 피드백을 받았다.
Names participatns = new Names(persons);💡 저는 Names로 일반화하고 불려지는곳에서의 이름으로 역할을 설명할것 같아요 😄
Participatns는 참가자명만 관리하는 일급 컬렉션이라서 Names로 일반화를 적용하였다.
@Test
@DisplayName("성공 - 참여자의 이름을 쉼표로 구분한다.")
void success_string_parse() {
String text = "pobi,honux,crong,jk";
int size = 4;
List<String> persons = StringParser.split(text);
assertThat(persons).hasSize(size);
}쉼표(,)를 기준으로 문자열 분리 하는 테스트 코드이다.
split된 후 크기만 검증하였다.
아래와 같은 피드백을 받았다.
assertThat(persons).isEqualTo(List.of("pobi", "honux", "crong", "jk"));💡 저는 이렇게 할것 같아요 🤔 객체간의 비교를하면 전반적으로 모든 조건을 테스트 할 수 있을것 같아서요 😄
크기 검증이 아닌 객체간의 비교를 하여 검증하는게 좀더 꼼꼼한 테스트지 않을까 하여 적용 하였다.
Step3 사다리(게임 실행)
프로그래밍의 요구 사항으로 '3개 이상의 인스턴스 변수를 가진 클래스를 쓰지 않는다.' 가 있었다.
해당 조건을 만족시키는게 많이 까다로웠다.
public enum Direction {
LEFT(-1),
STAY(0),
RIGHT(1);
private final int value;
Direction(int value) {
this.value = value;
}
public static Direction of(int value) {
return Arrays.stream(Direction.values())
.filter(direction -> direction.getValue() == value)
.findFirst()
.orElseThrow(() -> new IllegalArgumentException("숫자 값에 따른 Direction을 찾을 수 없습니다."));
}
//..
}
사다리는 왼쪽, 가운데, 오른쪽으로 움직일 수 있었다. Direction enum을 사용하여 이를 관리하였고, int 숫자값에 따른 Direction enum을 반환하고 있었다.
아래와 같은 피드백을 받았다.
.filter(direction -> direction.sameValue(value))💡 값을 꺼내오기 보다는 객체에게 물어보면 어떨까요?
사소한 내용이긴 하지만, 값을 꺼내 오는건 객체지향적인 코드가 아니다. 객체에게 메시지를 보내도록 코드를 작성하는 습관을 들이는게 좋다.
if (countOfPerson == 1) {
return new Line(List.of(STAY));
참여자가 한명일 경우 Line은 직선이면 된다고 생각을 했다. Line 객체는 List 타입의 인스턴스 변수를 가지고 있어서 List.of() 형식으로 리스트를 만들어서 넘기고 있었다.
아래와 같은 피드백을 받았다.
return new Line(STAY);💡 생성자를 다양하게 활용해볼것 같아요!
객체 생성시 부 생성자를 활용하면 할수록 좋다.
주의할 점은 부 생성자는 반드시 주 생성자를 호출해야 한다. 그래야 하지만 유효성 체크와 같은 중복 코드를 제거할 수 있다. 정적 팩토리 메소드도 마찬가지이다.
핵심은 중복 코드를 방지하고 테스트 코드의 유연성과 설계를 더 간결하게 만들어 유지보수성이 향상되는 점이다.
아래와 같이 구현하였다.
public class Line {
private final List<Direction> directions;
// 부 생성자
public Line(int[] directions) {
this(Arrays.stream(directions)
.mapToObj(Direction::of)
.collect(Collectors.toList())
);
}
// 주 생성자
public Line(List<Direction> directions) {
validateDirectionsContinuity(directions);
this.directions = new ArrayList<>(directions);
}
addDirection(countOfPerson, directions);
}
adjustDirections(directions);
return new Line(directions)
LineFactory에서 Line을 생성하는 로직이 있다. 모든 Line을 생성한 다음
사다리 규칙 중 하나인 ' 사다리 타기가 정상적으로 동작하려면 라인이 겹치지 않도록 해야 한다'를 확인하기 위해
모든 라인을 돌면서 수정하고 있었다.
아래와 같은 피드백을 받았다.
💡 addDirection(countOfPerson, directions);
참조 변수를 수정하는것은 별로 좋지 않은것 같아요.
전달하는 변수를 어딘가에서 수정하다보면 변수에 대한 신뢰도를 지키기 힘들고,
어디까지 사이드 이펙트가 전파될지 예측하기 힘들수 있기 때문이에요.
return new Line(countOfPerson);
생성에 대한 책임을 Line에게 주면 안될까요?
의문점이 있어서 질문을 하였다.
💡 LineFactory를 지우고 Line 생성에 대한 책임 로직을 Line 생성자로 옮겼습니다.adjustDirections() 메소드는 필요없는 것으로 생각되어 지웠습니다.
addDirection(countOfPerson, directions);이 부분은 Line 객체의 생성자 범위에서 수행하고 있으니 괜찮을 것 같은데.. 어떻게 생각하시는지 궁금합니다!!
참조 변수를 전달한 이유는 메소드 추출로 createDirection 메소드에 if문이 있어 메소드 depth를 줄이기 위함 입니다..!
adjustDirections 메소드는 만들어진 라인을 다시 수정하는 것이기에 추적이 힘들고 사이드 이펙트가 전파될 수 있다는 건 공감이 되었다. 참조 변수 수정에 대한 피드백은 directions 이라는 컬렉션 타입의 참조 변수를 다른 메소드로 넘겨서 피드백 주신것 같다. 하지만 메소드 depth를 줄이기 위해선 어쩔 수 없다고 생각했다.
아래와 같은 질문을 받았다.
💡 참조변수를 수정하는것에 대해서 어떻게 생각하시나요?
조금은 번외의 질문으로, 불변객체를 사용하면 어떠한 장점이 있을까요?
아래와 같이 답변하였다.
💡 해당 메소드에서 기준으로는 인자로 받는 참조변수를 수정하되 메소드에서 최종 반환하는 결과값의 의도(사다리 최종 결과)만 맞다면 괜찮지 않을까 조심스럽게 생각해봅니다..!
제가 아는 불변객체 장점은 아래와 같습니다!!
- 객체 생성 이후 객체의 상태를 변경할 수 없어 Thread safe
- 값 수정을 할 수 없어 사이드 이펙트 감소
- A 개발자가 만든 값을 B 개발자가 수정하면 디버깅 시간 소요
아래와 같은 답변을 받았다.
💡 우선, 피드백에 대해 깊은 고민을 해주셔서 진심으로 감사드립니다 😄
저는 불변객체를 그냥 심플하게 정의하는 편입니다.
비즈니스 로직을 예상가능하게 만드는데 도움이 된다.
우선 코드로 예시를 들어볼께요.
@Test
void test() {
Money 백원 = new Money(100);
Wallet 지갑 = new Wallet();
지갑.blabla(백원);
assertEquals(백원, new Money(100));
}
💡 해당 테스트는 성공할까요?
그냥 직관적으로 본다면, 비교값과 예상값이 백원이므로 당연히 성공해야겠죠.로직상에서도 큰 이상없어 보이구요.
하지만, 지갑에서 자기 마음대로 값을 변경하면 어떻께 될까요?
public void blabla(Money money) {
money.minus(10);
}
💡 외부에서는 값을 변경할지 몰랐지만, 참조된 변수를 수정함으로 인해서 디버깅 하기가 힘들어 지겠네요.
값이 변경되면 변수명도 수정해줘야하는거 아닌가요?
저는 이러한 사소한 부분을 내가 만든 로직을 오용해서 쓰이지 않기위한 노력이라고 봐요.
저는 참조변수 수정은 안티패턴이라고 생각하기 때문에 사용하지 않을것 같습니다.
이것은 저의 훈련방식중의 하나인데요.저 스스로가 정한 원칙을 위반하지 않기 위한 노력이라고 봐주시면 될것같습니다.습
관은 무섭기 때문이죠.그리고 저의 객체지향 사고방식을 이루는데에 방해가되기 때문입니다. 🙏
(동료 여러 개발자와 일을하거나 혹은 제가 리뷰하면서 느끼는것은 나는 당연히 이렇게 쓰이길 바랬는데 다른 개발자 분이 저의 코드를 가져다 쓸때는 오용해서 쓰는 경우가 종종 있더라구요.)
추가로 생성자나 메소드의 모든 파라미터에 습관적으로 final을 붙이 시는지 물어봤는데 그렇지 않다고 답변을 받았다.
최종적으로 Line을 생성하는 로직 아래와 같다.
public class Line {
private final Directions directions;
public Line(int[] directions) {
this(Arrays.stream(directions)
.mapToObj(Direction::of)
.collect(Collectors.toList())
);
}
public Line(Direction direction) {
this(List.of(direction));
}
public Line(DirectionStrategy directionStrategy, int countOfPerson) {
this(createDirections(directionStrategy, countOfPerson));
}
public Line(List<Direction> directions) {
this.directions = new Directions(directions);
}
private static List<Direction> createDirections(DirectionStrategy directionStrategy, int countOfPerson) {
List<Direction> directions = new ArrayList<>();
while (directions.size() < countOfPerson - 1) {
createDirection(directionStrategy, countOfPerson, directions);
}
return directions;
}
Line 객체의 생성자에서 자기 자신의 상태값을 생성할 수 있도록 수정 하였다.
createDirections 메소드 내에서 directions 참조 변수를 createDirection()에 넘겨주는데 객체 생성시는 괜찮지 않을까 싶은 생각이다.
public class Ladder {
//...
public int climb(int position) {
for (Line line : lines) {
position += line.move(position).getValue();
}
return position;
}
유저마다 climb 메소드를 호출한다
사다리의 라인을 한줄씩 탈때마다 바뀌는 결과값을 position에 반영하여 최종 위치의 결과 값을 반환한다.
아래와 같은 피드백을 받았다.
public Direction climb(int position) {
Direction direction = Direction.of(position);
for (Line line : lines) {
direction = direction.plus(line.move(position));
}
return direction;
}
💡 많은 부분 수정해야 할지도 모르지만, 저는 이렇게 도전해볼것 같네요 🤔
제공해주신 로직은 내가 작성한 도메인과는 맞지 않다.
- 인자로 건네주는 position 값은 참여자의 시작 위치(4명이면 0,1,2,3), but 코드상 Direction.of(positon) 할시 2,3일경우 exception 발생
- Directon 값은 -1,0,1 만 존재하는데 climb 메소드의 반환 값으로 Direction 반환 불가
최죙적으로 아래와 같이 수정 하였다.
public int climb(int position) {
for (Line line : lines) {
position = line.move(position);
}
return position;
}
position 값을 += 하지 않고 line 객체에 메시지를 보내서 사다리를 타면서 라인별 결과값을 받도록 수정 하였다.
Step4 사다리(리팩터링)
3단계에서 많은 피드백을 받아서 4단계에서 특별한 피드백은 없었다.
private static Line createLine(DirectionStrategy directionStrategy, int countOfPerson) {Ladder 객체 생성자에서 Line을 생성하는 메소드의 인자로 인원수를 받았다. 인원수 만큼 라인길이를 생성해야 하기 때문이다.
아래와 같은 피드백을 받았다.
private static Line createLine(DirectionStrategy directionStrategy, int columnSize) {💡 관점에 따라 다르겠지만, 호출하는쪽에서의 이름이 아닌 객체의 입장을 고려해서 일반화된 이름을 사용할것 같 습니다. 그렇게해야, 다른 개발자가 보더라도 흐름을 금방이해할수 있을것 같아서요~ 🤔
객체의 입장에서 보면 Ladde는 참여자 인원수, Line은 열 사이즈이다.
나의 생각은 반대인게 columnSize라 하면 객체가 아닌 서비스 파악에 조금 어려움이 있지 않을까 싶기도 하다.
참고
JAVA Optional의 충격적인 사용법 - map을 이용한 체이닝
[우아한테크코스] 사다리 타기 미션 회고(feat. TDD) 👍
'외부활동 > TDD, 클린 코드 with Java 17기' 카테고리의 다른 글
| TDD, 클린 코드 with Java 17기 종료(모든 미션 수료) (0) | 2023.12.13 |
|---|---|
| 수강신청 - 레거시 코드 리팩터링 회고 (0) | 2023.11.25 |
| 로또 - TDD 회고 (0) | 2023.11.11 |
| 자동차 경주 - 단위 테스트 회고 (1) | 2023.10.30 |
| TDD, 클린 코드 with Java 17기 시작 (0) | 2023.10.30 |