Step 1: 문자열 덧셈 계산기, 자동차 경주 구현

[PPakSang]

안녕하세요 남윤서 리뷰어님!

1주차 문자열 덧셈 계산기, 자동차 경주 구현 미션 리뷰 요청 드립니다☺️

페어와 함께 다음 컨벤션을 지키며 개발했습니다!

질문

일급 콜렉션을 생성할 때 외부에서 콜렉션을 주입받습니다.

public Numbers(List<Integer> numbers) {
   this.numbers = numbers;
}

• 이 경우에 외부에서 주입한 객체(List numbers) 가 노출(외부에서 수정)되는 문제가 있을 것 같은데 큰 문제가 없는건가요??

테스트 하기 쉬운 코드를 어떻게 작성해야할지 아직 잘 모르겠습니다.

• 프로덕션 코드에서 private method 로 추출해야 하는 경우 코드를 테스트하기가 어려웠습니다. 그렇다고 private method 로 추출하지 않을 수는 없었는 경우가 있는데 이런 경우에는 코드를 어떻게 작성하면 좋을까요?

• 아직까지 테스트코드 작성이 미숙해서 코드 리팩토링 시 테스트 코드를 변경해야 하는 경우가 많습니다,, 자연스러운 걸까요??

• 테스트코드만을 위한 객체 설계(ex. 요구사항에서는 차량의 위치가 항상 0 에서 시작이지만, 테스트를 위해 차량의 위치를 임의로 변경하기 위해서 외부에서 주입받는 생성자 혹은 메소드 추가) 를 해도 괜찮은 걸까요?

• 이 외에도 TDD 에서 프로덕션 코드를 크게 어떤 관점으로 작성해나가면 좋을지 조언 부탁드리겠습니다!

테스트 메소드 이름은 어떻게 작명하는게 좋을까요?

• 이번 미션에서는 Context_Result_ExceptionName 꼴로 이름을 작성했습니다.

도메인 객체가 외부로 노출되었을 때 발생하는 문제점(ex. 외부 레이어에서 너무 많은 도메인 지식을 알아야 한다, 도메인 로직 변경 시 그 영향이 외부로 퍼진다) 을 고려해서 VO 나 DTO 를 사용해서 그 영향을 최소화 하고자 했습니다. 올바른 방법인지 궁금합니다!

---

리뷰 감사합니다 😊

[begaonnuri]

#4747 (comment) 에 대한 답변입니다.

Q. 이 경우에 외부에서 주입한 객체(List numbers) 가 노출(외부에서 수정)되는 문제가 있을 것 같은데 큰 문제가 없는건가요??

말씀해주신건 오히려 일급컬렉션을 사용하지 않았을때 더 문제가 됩니다. List�와 Cars 클래스가 있다면 어떤 클래스가 더 조작하기 쉬울까요? Cars로 감싸서 변경되지 않도록하고 의도된 API만 public으로 열어줌으로써 의도한 동작을 수행할 수 있습니다. 일급 컬렉션에 대해 조금 더 학습해보시면 좋을 것 같아요.

Q. 프로덕션 코드에서 private method 로 추출해야 하는 경우 코드를 테스트하기가 어려웠습니다. 그렇다고 private method 로 추출하지 않을 수는 없었는 경우가 있는데 이런 경우에는 코드를 어떻게 작성하면 좋을까요?

private 메서드만 테스트하시려는 이유가 있을까요? 어떤 클래스를 사용하는 클라이언트는 private 메서드를 직접 사용할 일이 없고 public 메서드를 통해 간접적으로 사용합니다. 그렇다면 public 메서드가 잘 동작한다면 private 메서드의 동작은 자연스레 테스트되지 않을까요?

Q. 아직까지 테스트코드 작성이 미숙해서 코드 리팩토링 시 테스트 코드를 변경해야 하는 경우가 많습니다,, 자연스러운 걸까요??

테스트코드보단 TDD를 진행하는 어려움인것 같아요. 말씀하신 부분은 현업에서 일하시는 개발자분들도 많이 어려워 하는 부분입니다. TDD 사이클에 익숙해지시면서 점점 개선되겠지만 코드를 작성하기 이전에 구조에 대해 먼저 생각해봐도 좋을 것 같아요.

Q. 테스트코드만을 위한 객체 설계(ex. 요구사항에서는 차량의 위치가 항상 0 에서 시작이지만, 테스트를 위해 차량의 위치를 임의로 변경하기 위해서 외부에서 주입받는 생성자 혹은 메소드 추가) 를 해도 괜찮은 걸까요?

일반적으로 테스트 코드를 위한 프로덕션 코드의 추가나 변경은 좋지 않습니다. 하지만 생성자의 경우 객체의 풍부한 API를 제공한다는 이유로 제공하기도 합니다.

Q. 테스트 메소드 이름은 어떻게 작명하는게 좋을까요?

그렇게 중요한 부분은 아니라서 본인만의 기준을 갖고 일관성 있게 작명하기만 한다면 크게 상관 없습니다.

Q. 도메인 객체가 외부로 노출되었을 때 발생하는 문제점(ex. 외부 레이어에서 너무 많은 도메인 지식을 알아야 한다, 도메인 로직 변경 시 그 영향이 외부로 퍼진다) 을 고려해서 VO 나 DTO 를 사용해서 그 영향을 최소화 하고자 했습니다.

문제점은 잘 알고 계신것 같은데 해당 문제점을 해결할 수 있도록 구현이 이루어지지 않았다고 생각해요. 그리고 자동차 경주 게임과 같은 규모의 애플리케이션에서 말씀해주신 문제점보다 그렇게 구현했을때 복잡도나 번거로움이 더 클 수도 있습니다. 이 부분은 리뷰 드린 코멘트를 통해 더 얘기해보면 좋을 것 같아요 🙂

[begaonnuri]

안녕하세요 상현님, 미션 잘 구현해주셨네요.
질문에 대한 답변과 몇가지 코멘트 남겼는데 확인해주시고 잘 이해되지 않는 부분이 있다면 DM주세요!

[begaonnuri]

요구사항을 정말 꼼꼼하게 작성해주셨네요 👍🏻

[begaonnuri]

Scanner 대신 저수준의 BufferedReader를 사용해주신 이유가 있을까요?

[begaonnuri]

외부에서 사용하지 않는 메서드로 보이는데 접근제어자를 더 좁은 범위로 변경하면 어떨까요?

그리고 애초에 of를 분리할 필요가 있을까요? from에서 진행하지 않고 분리하신 이유는 무엇인가요?

[begaonnuri]

외부에서 생성자를 사용하는것과 어떤 차이가 있나요? 정적 팩터리 메서드를 사용하는 기준이 있었을까요?

[begaonnuri]

run() 메서드인데 OutputView.printStartMessage 인게 어색하게 느껴지네요.

start(), run()을 분리하신 기준으로 용어도 일관성 있게 변경하면 어떨까요?

[begaonnuri]

Race의 역할은 무엇일까요? Car를 생성하는 로직이 Race에서 이루어지고있는데 적절한 책임을 가진 객체로 이동해보면 어떨까요?

[begaonnuri]

전반적으로 cars.getCars() 이후 로직이 진행되는데 이것은 Cars의 역할인 행동들을 Race에서 꺼내서 대신 수행하고있다는 신호이기도 합니다. 그 결과 Cars는 사실상 List<Car>를 감싸기만한 자료구조가 되어버린 것 같아요.

Cars의 책임에 맞는 로직들을 옮겨 자료구조가 아니라 역할과 책임을 갖는 객체로 만들어보면 어떨까요?

[begaonnuri]

Util 클래스의 역할은 무엇이라고 생각하시나요?

자동차의 움직임을 결정하는것은 자동차 경주 게임의 핵심 비즈니스 로직인데 Util 클래스로 분리되어있는게 어색하게 느껴지네요.

[begaonnuri]

NumberGenerator 인터페이스를 통해 랜덤값을 테스트할 수 있도록 작성해주셨네요 👍🏻

[begaonnuri]

Nested 클래스로 구성해주신 이유가 있을까요? 개인적으로는 파일의 길이가 길어 컨텍스트 파악이 어렵네요.

[begaonnuri]

그리고 매번 PR을 작성하시기 번거롭기도 하고 규모에 맞지 않는 부분도 있어서 PR 대신 커밋으로 진행해주세요.

진행하실땐 커밋 메시지 규약을 참고해주세요!

https://velog.io/@outstandingboy/Git-%EC%BB%A4%EB%B0%8B-%EB%A9%94%EC%8B%9C%EC%A7%80-%EA%B7%9C%EC%95%BD-%EC%A0%95%EB%A6%AC-the-AngularJS-commit-conventions