전체 보기
🪐

지식 동기화 동참 (코드리뷰를 통해 얻은 것)

작성일자
2023/11/09
태그
JAVA
프로젝트
PreCourse
책 종류
1 more property

시작하며

아래 글이 매우 인상 깊었습니다. 프론트분이신데 정보 격차 부분에 대해 공감이 갔고, 저도 제가 1~2주차의 코드 리뷰 속에 얻은 정보를 정리해 공유하고 싶어졌습니다. 그리고 작성을 시작한 이 글은,, 생각보다 정리에 정말 정말 오랜 시간이 걸렸습니다. 최대한 보편적인 시각을 담기 위해 거의 3~4일 가량 투자해 정리했지만, 혹시 다른 좋은 의견이 있거나 틀린 정보가 있다면 언제든 댓글 남겨주면 감사하겠습니다.

 궁금했던 것

일급 컬렉션은 본인이 감싸고 있는 콜렉션을 반환해도 될까?

결론부터 말하자면 된다. 일급 컬렉션의 정의가 중요한데 일급컬렉션이란 Collection을 Wrapping하면서, 그 외 다른 멤버 변수가 없는 상태를 말한다. 그렇다면 여기서 Collection은 어떤 것들이 있을까? 우리가 흔히 쓰던 List, Set, Map, Queue를 이야기할 수 있다.
그럼 일급컬렉션에서 본인이 Wrapping하고 있는 Collection을 반환해도 될까? 되긴 된다. 하지만 주의할 점이 있다. Collection의 특징을 생각해보면 좋은데, 예를 들어 List의 경우 add로 충분히 다른 곳에서 조작가능하다. 따라서 일급 컬렉션에서 Collection을 반환하되 Collections.unmodifiableList(list)와 같이 감싸 반환해야 한다. 이렇게 보내면 다른 곳에서 add를 막아 Collection을 일급 컬렉션 외에 다른 곳에서 조작하는 걸 막아줄 수 있다.
마무리하며 일급 컬렉션에서 컬렉션 객체를 다룰 때 특징을 정리하자면 아래와 같다.
컬렉션 객체를 변수나 매개변수에 할당할 수 있고, 반환값으로도 사용할 수 있음
컬렉션 객체는 필요한 경우 메서드에서 생성할 수 있음
컬렉션 객체는 다른 객체와 동등한 지위 지님

 DTO 매핑은 누구의 책임일까?

더 엄밀히 나눠서 매핑과 생성 각각에 대해 논하겠다. 예를 들어 Model 내용을 바탕으로 Dto를 생성할 일이 있다 해보자. 이에 대해 다음과 같은 질문을 던질 수 있다.
Model 내용을 Dto로 매핑하는 작업은 어디서 해야할까?
질문에 대한 답은 아키텍처 구조에 따라 달라질 수 있기에 정해진 답은 아니지만, 이 경우엔 dto에서 매핑 작업하는 걸 권장한다 답하고 싶다. 그 이유론 도메인이 dto에 의존하는 것보단 dto가 도메인에 의존하는 게 더 낫기 때문이다. dto가 도메인에 의존한다는 건 dto가 도메인을 이용한단 것인데 코드로 보여주자면 아래와 같다
도메인이 dto에 의존 (도메인이 dto를 이용)
public class Car { public CarState summarizeState() { return new CarState(this.name, this.forwardCount); }
Java
복사
dto가 도메인에 의존 (dto가 도메인을 이용) → 권장
public record CarState(String name, int moveCount) { public static CarState capture(Car car) { return new CarState(car.getName(), car.getMoveCount()); } }
Java
복사
dto가 도메인에 의존하는 경우 도메인은 dto의 변경에 영향을 받지 않고, dto만 도메인의 변경에 영향을 받는다. 이게 좋은 이유는 도메인과 dto의 역할이 다르기 때문이다. dto는 모델의 상태를 스냅샷하는 도구로써 모델의 변경에 영향을 받는 게 타당하지만, 모델은 순수성을 보장하기 위해 dto의 변경에 영향을 받지 않는 것이 좋다.
또한 dto의 추가는 쉽게 예상 가능한 변화인데, 도메인이 dto에 의존하는 경우엔 dto가 추가될 때마다 도메인에 dto로 변환해주는 메서드를 추가하게 되어 확장이 기존 소스의 변화를 유도하는 ocp 위반이다. CarState 외에 수십 개의 레코드 타입 DTO가 만들어질 경우 원본 객체 클래스가 변환 로직으로 더러워질 가능성이 크단 뜻이다.
결론적으로 원본 객체(model)에서 dto로의 매핑이 필요한 경우, 원본 객체를 dto로 매핑해오는 로직은 dto 클래스 안에 존재함으로써 원본 객체는 매핑 로직을 모르는 편이 좋다. 모델이 dto의 구조에 의존하지 않는 것이다.

 Application은 Controller의 함수를 얼마나 알아야 할까?

여기에 대해선 나도 고민이 많았고 아래와 같이 코드를 짜보았다. 좋은 의견이 많이 달렸는데 대화의 흐름을 요약해보겠다.
public class Application { public static void main(String[] args) { // TODO: 프로그램 구현 RacingController controller = new RacingController(new InputView(), new OutputView()); controller.createRacing(); controller.playRacing(); controller.finishRacing(); } }
Java
복사
B와 D는 기존 코드 유지를 주장하고, A와 C는 코드를 아래와 같이 바꾸는 걸 주장하고 있다
public class Application { public static void main(String[] args) { // TODO: 프로그램 구현 RacingController controller = new RacingController(new InputView(), new OutputView()); controller.run(); // run안에서 createRacing, playRacing, finishRacing을 모두 호출함
Java
복사
A : Main이 컨트롤러를 올바르게 호출할 책임을 갖는 것보단, 컨트롤러가 경주에 대한 컨트롤을 쥐는 것이 캡슐화 측면에서 좋을 거 같다.
B : Application의 책임을 시스템 그 자체로 생각하지 않고, 핸들러 객체의 실행을 담당하는 객체로 생각하면 지금의 코드도 좋을 거 같다.
C : 비즈니스 로직 길어짐에 따라 Application의 로직 또한 길어질텐데, Application의 책임이 너무 커지고 프로그램 동작에 대해 이해하기 어려워질 거 같다. 따라서 Application 객체에는 프로그램 실행에 필요한 최소한의 로직만 작성하는 게 좋을 거 같다
D : createRacing과 playRacing, finishRacing을 api처럼 보면 어떨까 싶다. Racing이란 도메인을 생성하고 수정하고 하던 일련의 과정을 표현한 것으로 보는 거다. 즉, main이 api를 사용하는 쪽인 프론트엔드 역할인 것이다.
A : 메인 스레드의 역할과 책임을 어디까지로 볼 것이냐에 따라 다른 역할을 수행할 수 있는 건 맞다. 하지만 메인 메서드는 static 키워드에서 알 수 있듯이 애초에 객체가 아니다. main이라는 이름을 가진 정적 메서드인 것이다. main 메서드가 가지는 의미는 프로그램의 엔트리포인트로서의 역할이다. 즉, 프로그램을 시작할 때 뭔가 필요한 것이 있으면 이런 저런 작업을 해주는 것이 적절한 역할인 거이다.
나는 이를 읽고 A와 B의 주장에 동의하게 되었다. 그 이유는, main이 정적 메서드기에 엔트리포인트로서의 역할을 한다는데에 동의했기 때문이다. controller가 여러 객체로 더 갈기 갈기 찢어져서 그 객체들이 서로 소통하지 않는, 한 하지과 같이 controller가 하나라면 main이 컨트롤러 내부 메서드를 다 알 필욘 없어 보인다. 특히 캡슐화 측면에서도 말이다.

 전략 패턴은 어느 시점에 적용해야 오버킬이 아닐까?

전략 패턴은 객체의 행동을 런타임에 바꿀 수 있도록 해주는 디자인 패턴으로, 특정 알고리즘을 인터페이스로 정의하고 런타임에 적절한 구현을 선택하여 사용할 수 있게 해준다. 예를 들면 우승자 결정 기능을 어떤 경우엔 가장 높은 점수를 가진 이를 우승자로 결정할 수도 있고, 어떤 경우엔 가장 낮은 점수를 가진 이를 우승자로 결정할 수도 있는데 이들을 각각의 전략으로 두고 각각 필요한 상황에서 교체해주는 거다.
이번 과제에선 전략 패턴이 떠오르는 곳은 크게 두 곳이다.
1.
MoveStrategy : 자동차가 이동할 때 사용
2.
WinnerStrategy : 우승자를 결정할 때 사용
어느 곳에 적용하는 게 오버킬이 아닐까? 오버킬은 YAGNI 기준으로 말하겠다. YAGNI란 “개발 하다보면 당장 필요하지느느 않지만 확장성을 위해 미리 작업하는 경우가 있는데 이런 작업을 하지 마라”란 뜻이다.
일단 WinnerStrategy는 YAGNI 원칙에 위반되는 게 맞다. 왜냐하면 이번 요구사항에서 우승자 결정 로직은 한 가지만 언급되었기 때문이다. 추후 확장성을 위해 전략패턴을 적용할 수도 있겠지만 YAGNI 원칙에 따르면 하지 않는 편이 맞다
그렇다면 MoveStrategy는 어떨까? 이 역시 이번 요구사항에서 자동차 이동 시 자동차의 전진 여부를 결정하는 로직이 한 가지만 언급되었기 때문에 YAGNI 원칙에 위반하는 것처럼 보인다. 하지만 이는 WinnerStrategy와 다르게 전진 여부 결정 로직이 랜덤 숫자에 의존하기에 테스트 코드에선 다른 전략이 필요하다는 점이 특이하다. 테스트는 랜덤 숫자에 의존해선 안되기 때문이다.
이로 인해 많은 사람들이 전략 패턴까진 아니더라도 인터페이스를 만들어 사용하거나 하는 식으로 테스트코드에서 사용하는 전진 여부 결정 로직과 프로덕션 코드에서 사용하는 전진 여부 결정 로직을 구분해 줬으리라 생각한다. “이 같은 구조가 YAGNI 원칙을 위반하는가?”란 질문에 대한 답으로 “아니다. 테스트의 정확성을 확보하기 위해 필수적인 결정으로 필요한 설계 결정이다.”라고 답하고 싶다.
더 나아가 전략 패턴을 적용할 수도 있을 것이다. 전략 패턴을 적용한 건 코드로 보여주겠다. 이 경우엔 전략패턴을 적용한 것 역시 인터페이스 기반 구현과 마찬가지로 오버킬이 아니다.
public class Car { .. public void move(MoveStrategy moveStrategy) { if (moveStrategy.isMove()) { position++; } }
Java
복사
public interface MoveStrategy { boolean isMove(); } public class RandomMoveStrategy implements MoveStrategy { private static final int MOVABLE_THRESHOLD = 4; private final RandomGenerator randomGenerator; public RandomMoveStrategy(RandomGenerator randomGenerator) { this.randomGenerator = randomGenerator; } @Override public boolean isMove() { return randomGenerator.generate() >= MOVABLE_THRESHOLD; } }
Java
복사
public class CarTest { void move() { car.move(()->true); // MovingStrategy가 메서드 한 개인 Functional Interface이므로 람다형으로 사용함 assertThat(car.getPosition()).isEqualTo(1); } void stop() { car.move(()->false); assertThat(car.getPosition()).isEqualTo(0); } }
Java
복사
위 코드는 리팩토링 과정에서 참고한 블로그에서 일부 발췌한 코드입니다. 더 자세한 내용이 보고 싶으시다면 https://jackjeong.tistory.com/entry/Java-Strategy-Pattern전략패턴feat-Interface 에 들어가서 확인해주심 좋을 거 같습니다.
CarsTest에 대해 의문이 나올 수 있다. 테스트 시 각각의 자동차들이 모두 같은 이동 전략을 취하게 되진 않을까 염려될 수 있기 때문이다. 이에 대해선 아래와 같이 구현해 해결해줄 수 있다.
public class SequentialMoveStrategy implements MoveStrategy { private Iterator<Boolean> movesIterator; public SequentialMoveStrategy(List<Boolean> moves) { this.movesIterator = moves.iterator(); } @Override public boolean isMove() { return movesIterator.hasNext() && movesIterator.next(); } } public class CarsTest { ... @Test void 예시코드입니다_활용해서_다양한_테스트코드를_작성해주세요() { // 이동 시도 1회 private List<Boolean> moves = Arrays.asList(true, false, true); private MoveStrategy moveStrategy = new SequentialMoveStrategy(moves); cars.move(moveStrategy); // 이동 시도 2회 moves = Arrays.asList(false, true, false); moveStrategy = new SequentialMoveStrategy(moves); cars.move(moveStrategy);
Java
복사

 뷰에서 처리할 예외와 모델에서 처리할 예외를 어떻게 구분할까?

mvc 패턴이라 전제하고 예외 처리(검증 로직)를 어디서 해야 할지 논의했다. 물론 사람마다 다르겠지만 내가 내린 결론은 아래와 같다. 내가 정답은 아니기에 이건 참고 정도만 해도 좋을 거 같다.
뷰와 모델이 가질 검증 책임을 각각 어디까지로 둘지 생각해보았다. 그리고 아래와 같이 나눠줬다.
뷰 : 타입 변환 중 생기는 예외 처리 (String ->intString -> List<String> 등)
모델 : 비즈니스 로직과 관련한 예외 처리 (횟수가 1회 이상인지이름이 5글자 이하인지중복된 이름인지 등)
사실 비즈니스 로직과 관련한 예외처리를 어디까지로 볼지 애매한데 나는 api 개발할 때를 떠올렸다. 보통 requestBody의 각 필드에 대해 아무리 유효성 검증을 안한다해도 타입은 지정해두고 받아온다.
그래서 뷰의 역할도 마찬가지로 단순히 문자열을 받는 거에서 더 나아가 딱 타입 변환까지만 확장해줬다.

 절차지향을 무조건 지양해야할까?

객체지향만을 고려하는 건 100kg한테 small 사이즈를 입히는 것과 같을 수 있다. 라는 한 리뷰어의 의견이 인상 깊었다. 아래 두 코드를 보고서 어떤 게 더 객체지향적인지 생각해보자.
1번 코드
public class RacingController { private final InputView inputView; private final OutputView outputView; private Racing racing; public RacingController(InputView inputView, OutputView outputView) { this.inputView = inputView; this.outputView = outputView; } private void createRacing() { List<String> carNames = inputView.askCarNames(); int tryCount = inputView.askTryCount(); this.racing = new Racing(carNames, tryCount, new RandomIntGenerator()); } private void playRacing() { outputView.printRacingAnnouncement(); while (racing.canMove()) { racing.doMove(); List<CarState> states = racing.captureCurrentState(); outputView.printAllCarPositionByState(states); } } private void finishRacing() { List<String> winnerCarNames = racing.generateWinnerNames(); outputView.printWinnerCar(winnerCarNames); } } public class Application { public static void main(String[] args) { // TODO: 프로그램 구현 RacingController controller = new RacingController(new InputView(), new OutputView()); controller.createRacing(); controller.playRacing(); controller.finishRacing(); } }
Java
복사
2번 코드
public class RacingController { private final InputView inputView; private final OutputView outputView; public RacingController(InputView inputView, OutputView outputView) { this.inputView = inputView; this.outputView = outputView; } public void run() { Racing racing = createRacing(); playRacing(racing); finishRacing(racing); } private Racing createRacing() { List<String> carNames = inputView.askCarNames(); int tryCount = inputView.askTryCount(); Racing racing = new Racing(carNames, tryCount, new RandomIntGenerator()); return racing; } private void playRacing(Racing racing) { outputView.printRacingAnnouncement(); while (racing.canMove()) { racing.doMove(); List<CarState> states = racing.captureCurrentState(); outputView.printAllCarPositionByState(states); } } private void finishRacing(Racing racing) { List<String> winnerCarNames = racing.generateWinnerNames(); outputView.printWinnerCar(winnerCarNames); } } public class Application { public static void main(String[] args) { // TODO: 프로그램 구현 RacingController controller = new RacingController(new InputView(), new OutputView()); controller.run } }
Java
복사
내 원래 코드가 1번 코드였고, 개선한 코드가 2번 코드다. 1번 코드와 2번 코드의 가장 큰 차이점은 1번은 RacingController가 Racing 상태를 가지는 것이고, 2번은 RacingController의 메서드들이 Racing을 메시지처럼 주고 받는 것이다. 1번처럼 코드를 짰던 이유는 절차지향을 회피하고 싶어서였다. 즉, start, paly, finish 순서로 함수를 호출하는 순차적인 부분을 Controller에서 지우고 싶었던 것이다.
하지만, 아이러니하게도 캡슐화, 추상화, 단일책임, 모듈화 등 모든 측면에서 2번 코드가 더 객체지향적이다. 예를 들어 Controller가 여러 개의 Racing을 처리해야 하게 요구사항이 변경된다면 2번은 훨씬 유연하게 처리할 수 있을 것이다. 여기서 깨닫게 된 점은 순차적 호출을 무조건 피한다고 해서 객체지향적인 코드가 만들어지는 게 아니란 점이었다. 객체지향이란 게 애초에 절차지향과 반대되는 개념이 아니기에, 순차적 호출을 무조건적으로 지양할 필요는 없다.
더 급진적인 예시로 아래 강연에서 때론 객체지향보단 절차지향적으로 가는 게 나았던 케이스를 언급한다. 물론 나는 내용이 어려워 전부 이해하진 못했다 ㅎㅎ 그저 흐름만 느꼈을 뿐,, 궁금한 사람들은 강연을 봐보는 걸 추천한다!

 테스트가 어렵다면 코드를 바꾸는 게 좋을까?

테스트 하기 좋은 코드여야 설계가 잘 되어 있단 뜻이다. 라는 말이 있다. 우리가 구현을 마치고 테스트 코드를 짜려는데 기존 코드를 바꾸지 않으면 테스트를 짤 수 없는 상황을 마주할 때가 있다. 이땐 어떻게 해야 합리적인 대처법이라 할 수 있을까? 이에 대해 나는 아래와 같이 정의하고 싶다.
1.
테스트만을 위한 코드를 프로덕션 코드에 추가하는 건 막자
2.
확장성을 고려 못한 코드라고 판단되면 프로덕션 코드를 수정할 수 있다
→ 이 경우 테스트를 위해 코드 바꾸는 순간 앞뒤가 바뀌는 거 아니에요? 라고 물을 수도 있다. 하지만 내가 말하는 코드 수정은 프로덕션 코드를 확장성 있게 리팩토링하는 것까지만 한정한다고 답하고 싶다.
이번 2주차 과제에서 아마 많은 사람들이 동시에 고민했을 부분이 랜덤 관련한 부분일 거 같다. 자동차가 이동할지 말지를 결정할 때 프로덕션 코드에선 랜덤 숫자를 이용하는 반면, 테스트 코드에선 랜덤 숫자를 이용하지 않기에 나 역시 그 차이를 메꾸는 데에서 고민했다.
처음엔 숫자를 생성하는 인터페이스를 만들고, 랜덤 숫자를 생성하는 클래스와 테스트용 숫자를 생성하는 클래스 두 개가 이를 구현하게 만들어줬다. 이 코드의 문제점이 뭘까? 테스트 용이성만을 위해 코드(클래스)가 추가됐단 점이다.
이를 해결하기 위한 간단한 방법으론 Stub을 사용해 적어도 테스트용 숫자를 생성하는 클래스를 프로덕션 코드에서라도 분리해주는 것이다. 그렇게 되면 1번 규칙을 지킬 수 있다. 이에 대해선 바로 다음 문항에 적어두었다.
또 다른 방법으론 2번 규칙을 적용한 것으로 전략 패턴을 사용하는 것이 있다. 이때, 전략 패턴을 프로덕션 코드엔 적용하지 않고 테스트코드에만 적용하는 것이다. 그렇게 되면 프로덕션 코드에선 랜덤 숫자를 생성하는 게 고정되는 태초의 설계대로 구현될 것이고, 테스트코드에선 테스트용 숫자를 생성하거나 다른 방식으로도 숫자를 생성할 수 있게 유동적인 전략패턴을 적용한 설계가 적용될 것이다.
public class Car { private final String name; private final MoveStrategy moveStrategy; private int position = 0; public Car(String name) { validateName(name); this.name = name; this.moveStrategy = new RandomMoveStrategy(); // 프로덕션에서 사용 } public Car(String name, MoveStrategy moveStrategy) { validateName(name); this.name = name; this.moveStrategy = moveStrategy; // 테스트 시 사용 }
Java
복사
//test static class FixedMoveStrategy implements MoveStrategy { private List<Boolean> movables; private int index = 0; public FixedMoveStrategy(List<Boolean> movables) { this.movables = movables; } @Override public boolean determineMovable() { if (index >= movables.size()) { return false; } return movables.get(index++); } } ... Car car = new Car("test", new FixedMoveStrategy(List.of(true, false, true)));
Java
복사

 새로 알게 된 것

 Stub, FunctionalInterface 등 을 이용해 테스트코드를 위한 코드 만들 수 있다

Stub이란 원래의 구현을 단순한 것으로 대체하는 것을 말한다. 즉, 테스트에 맞게 단순히 원하는 동작을 수행하게 해주는 것이다. 원래 프로덕션 코드에선 랜덤 숫자를 만들어주던 반면, 테스트 코드에선 단순히 iteration을 돌려 숫자를 만들게 하던가, 아님 생성자에서 들어온 숫자 그대로를 뱉게 해주던가 하는 식으로 사용할 수 있다. 주의할 점은 아래 stub들은 프로덕션 패키지가 아닌 테스트 패키지에 넣어주자.
또 , 더 나아가서 stub에서 하는 일이 단순히 들어온 숫자를 그대로 뱉는 일이라면 함수형 인터페이스를 사용하는 것도 추천한다. 따로 구현체 만들지 않고도 람다로 () -> List.of(1, 2, 3, 4, 5, 6)와 같이 사용할 수 있기 때문이다.
public class Car { private final String name; private final IntGenerator intGenerator; private int position = 0; public Car(String name, IntGenerator intGenerator) { validateName(name); this.name = name; this.intGenerator = intGenerator; }
Java
복사
iteration 돌려 반환하는 코드
//test public class SequentialIntGeneratorStub implements IntGenerator { private final List<Integer> numbers; private int index = 0; public SequentialIntGeneratorStub(List<Integer> numbers) { this.numbers = numbers; } @Override public int pickNumber() { int number = numbers.get(index); index++; if(index >= numbers.size()) { index = 0; } return number; } } ... cars = Cars.createCars(carNames, new SequentialIntGeneratorStub(List.of(4,4,3)));
Java
복사
단순 숫자 반환하는 코드
//test public class CustomIntGeneratorStub implements IntGenerator { private final int number; public CustomIntGeneratorStub(int number) { this.number = number; } @Override public int pickNumber() { return number; } } ... cars = Cars.createCars(carNames, new CustomIntGeneratorStub(3));
Java
복사

 Java17부턴 긴 문자열을 가독성 좋게 쓸 수 있다 (특히 테스트코드에서)

원래라면 개행 문자(\n)를 넣었을 텐데 아래처럼 사용해줄 수도 있다.
String expectedOutput = """ car1 :\s car2 : - car3 : -- """; String actualOutput = outputStream.toString(); assertThat(actualOutput).isEqualTo(expectedOutput);
Java
복사