목차
1. GitHub 저장소
2. 구현 기능 목록
3. 기능 구현 과정
4. TDD 강의
5. 코드 리뷰
1. GitHub 저장소
리팩토링 과정을 눈으로 확인할 수 있도록 브랜치를 분리하며 진행했다.
프로토타입: https://github.com/yeon-06/java-racingcar/tree/step1
코드리뷰 반영: https://github.com/yeon-06/java-racingcar/tree/step2
Pull Request: https://github.com/woowacourse/java-racingcar/pull/274
2. 구현 기능 목록
- 입력 관련
- 자동차 이름을 입력받아 자동차 생성
- 쉼표로 구분
- 이름이 5자 이하인지 체크
- 중복 제거
- 빈 값이 있는지 체크
- 레이싱을 시도할 회수 입력
- 숫자인지 체크
- 양수인지 체크
- 자동차 이름을 입력받아 자동차 생성
- 게임 진행
- 0에서 9 사이의 랜덤한 값을 생성
- 랜덤값이 4 이상이면 자동차를 전진
- 매 시도마다 진행 상태 출력게임 결과
- 우승자 계산
- 우승자 출력
- 결과를 String으로 만들어서 출력
- 공동 우승 처리
기능은 프리코스 2주차 때 진행했던 자동차 경주와 동일하다.
🔻 프리코스와 다른 점
짝 프로그래밍으로 진행한 뒤 리뷰어 분들께 일대일 코드 리뷰를 받는다.
리뷰를 받고 진행하는 리팩토링 때는 짝과 별개로 진행한다.
짝 프로그래밍은 처음 겪어봤는데 보통 zoom과 code with me를 이용한다.
내 똥컴은 zoom과 IntelliJ 동시에 실행 시키는게 불가능해서... (내 노트북 나이 6살)
짝인 오찌가 날 배려해서 zoom 말고 디코로 통화했다. 감사합니다
3. 기능 구현 과정
프리코스와 크게 다를 바 없이 요구조건에 충실하게 구현했다.
다만 이번에는 TDD 방식으로 개발하게끔 노력했다.
구현할 기능 목록을 정리한 뒤 가장 작은 단위부터 생각해보았다.
이번 과제에서는 가장 작은 단위가 Car가 될 것이라 생각했고 바로 CarTest부터 생성했다.
@SuppressWarnings("NonAsciiCharacters")
class CarTest {
@Test
public void 자동차_생성() {
// given
String name = "test1";
// when
Car car = new Car(name);
// then
assertThat(car).isNotNull();
}
}
Car라는 클래스를 선언한 적이 없으니 당연히 빨간줄이 뜰 것이다.
이제 빨간줄을 없애기 위해 Car 클래스를 생성한다.
자세한 로직은 작성하지 않고 위 테스트를 통과할 수 있을 정도의 코드만 작성했다.
public class Car {
private final String name;
public Car(String name) {
this.name = name;
}
}
위에서 작성했던 테스트 코드를 실행시키면 실행이 완료될 것이다.
이제 테스트 코드를 추가해나가자.
자동차 이름과 관련해서는 5글자 이하에 빈 값이 불가능하다는 조건이 있었다.
@ParameterizedTest
@ValueSource(strings = {"", "123456", " "})
public void 자동차_생성_실패(String name) {
// then
assertThatThrownBy(() -> new Car(name)).isInstanceOf(IllegalArgumentException.class);
}
Car에 생성 시 이름을 검증하는 메소드를 생성했다.
public class Car {
// ...
public Car(String name) {
validate(name);
this.name = name;
}
private void validate(String name) {
if (name == null || name.isBlank() || name.length() > MAX_LENGTH) {
throw new IllegalArgumentException(ErrorMessages.EMPTY_NAME);
}
}
}
이런 식으로 아래 과정을 계속 반복했다.
- 테스트 코드 작성
- 빨간 줄 없애기 (로직 작성)
- 테스트 코드 실행
- 성공 시 또다른 테스트 코드 작성
4. TDD 강의
TDD가 익숙하지 않은 나같은 크루들을 위해 우테코 측에서 강의를 준비해주었다.
강의 중 메모 해둔 것들을 정리해둔다.
💡 Tips 💡
- final 키워드를 활용하자!
👉 부수 효과로 인한 오류 가능성 ↓ - private 메소드 테스트하기
👉 메소드 내용을 복사해서 테스트 클래스에 붙여넣고 테스트해도 ok - 핵심적인 비즈니스 로직 테스트하기
👉 존재하는 모든 메소드에 대해 테스트할 필요는 없음. 테스트의 목적을 생각할 것. - 커밋은 하나의 기능이 완료됐을 경우
👉 실패한 테스트나 컴파일 오류 등이 발생하면 안된다.
❓ 궁금한 것들 ❓
- ❓ 테스트 코드의 메소드명이 한글 지원 되는데 @DisplayName을 사용하는 이유?
👉 경고들이 쌓이면 로그가 남게 되는데 큰 프로젝트 등에서 너무 많이 쌓이게 되면 이것을 하나하나 확인하는 것도 큰 일이다.
💡 @SuppressWarnings(”NoneAsciiCharacters”) 사용하는 방법도 있음 - ❓ @DisplayName과 테스트 메서드 이름 사이에 어떤 차이를 주는게 좋을까?
👉 자유. 중복도 OK. (개인적으로) 테스트 메소드는 간단히 적고 @DisplayName에는 한글로 자세히 적는 편 - ❓ 한 테스트에서 assert 여러개 사용
👉 테스트 실패 시 어디에서 실패했는지 따로 확인해야 함. 반드시 여러개 사용해야 하는 경우 내가 검증할 것이 여러가지인건 아닌지 생각해보기. (여러가지면 테스트 메소드를 분리할 수 있으니까) - ❓ 당장은 사용하지 않지만 VO 취급을 위해 equals()랑 hashCode()를 오버라이드 해도 될지?
👉 괜찮다! 추가로 동일성과 동등성에 대한 차이를 알아보면 좋을 것 같다. - ❓ private 메소드도 테스트를 하고싶다면?
👉 테스트 코드를 위해 접근 제어자를 바꾸거나 Reflection을 활용하는 것은 좋은 방법이 아닌 것 같다. 설계나 로직을 다시 확인해볼 필요가 있을 듯.
+ http://shoulditestprivatemethods.com
5. 코드 리뷰
짝 프로그래밍을 마치고 PR을 제출했다.
구체적은 코드 리뷰들은 PR로 확인하는 것이 더 편리하니 하나하나 캡쳐하지는 않겠다.
생각할 여지가 있는 것, 개선해야할 점만 정리를 해보았다.
Notes
- 메소드 하나당 기능 하나만 담당하도록 잘 분리했는지 생각해보기
- 예외는 어디서 처리해야 할까?
- 각 클래스에 존재하는 메소드의 위치와 역할이 적절한지
👉 객체에서 값을 꺼내 쓰기보다는 값을 넣어서 객체 내에서 비교하는 형태로 만들기 - 충분한 테스트 케이스를 작성했는지 확인해보기
👉 예외의 상황을 다양하게 생각해볼 것 - toString()의 목적이 무엇일까?
👉 객체 내부의 정보를 간결하고 명확하게 String 형태로 표현
👉 https://yeonyeon.tistory.com/188
추가로 개발을 진행하며 궁금했던 점을 PR 올릴 때 작성했다.
내용이 너무 길어서 각 항목에 대한 내용과 받은 답변을 접는 글로 정리했다.
(🙋♀️: 본인 - 연로그, 🙍♂️: 리뷰어 - 카일)
🔻 @DisplayName vs @SuppressWarnings
🙋♀️❓ @DisplayName을 사용하는 이유가 메소드 명을 한글로 지정하는 경우 IntelliJ의 경고 문구로 인해 불필요한 로그가 쌓이는 일을 피하기 위해 라고 알고 있습니다. 저는 이름 짓는게 개발에서 매우 중요한 일인만큼 많은 시간을 소요한다고 생각합니다. 메소드마다 영어이름 + DisplayName에 붙일 한글이름 지정해주는게 번거로워서 @SuppressWarnings을 붙여서 오류를 피하는 방식을 택했는데 카일은 어떤걸 선호하는지, 그 이유는 무엇인지 궁금해요!
🙍♂️👉 너무 좋은 고민 포인트라고 생각해요. 일단 이것에 대해서는 연로그의 기준이 가장 중요할 것 같아요. DisplayName에는 어떤 것을 명시할 것이며 메소드 이름에는 무엇을 명시할 것이다 라는 기준이 있다면 그에 따르면 될 것 같구요. 디스플레이 네임으로 충분히 의도를 드러낼 수 있다면 그것만 사용하는 것도 좋은 방법인 것 같아요. 저는 개인적으로 메소드 네임은 랜덤하게 짓고 디스플레이네임만 사용하는 걸 선호합니다.
🙋♀️❗ 메소드 이름은 항상 의미있게 지어야 한다고 배워왔어서 테스트 코드에서도 적용되는 이야기인줄 알았는데 유도리 있게 저만의 기준을 세우며 적용하면 되겠군요! @SusppressWarnings 통해 억지로 에러를 무시하는게 마음에 걸렸는데 카일의 방법이 좋은 것 같아요.
🔻 Pattern의 필드화가 좋은 것일까?
🙋♀️❓ 메소드 내에서 Pattern을 사용하고 있습니다. Pattern의 생성 비용이 크다고 알고 있어서 메소드 내부에 선언하지 않고 전역 변수화 했습니다. 메소드를 호출할때마다 Pattern을 생성하기 보다 static final 키워드를 붙여 전역 변수로써 만들어 재사용하는 것이 메모리 절약도 되고 코드도 보기 편하다고 생각을 했는데 static의 사용을 남발하게 되는건 아닌지 우려가 됩니다. (너무 많은 걱정을 한다는 자각은 있지만.. static을 너무 자주 사용하는건 안좋다고 책에서 읽어서요😂) 카일은 이런 경우 어떻게 처리하시는지 궁금합니다!
🙍♂️👉 static을 너무 자주 사용하는건 안 좋다는 책을 보신 것 같은데 왜 안 좋은지에 대해서도 고민해보면 좋을 것 같아요🙂 좋지 않은 이유보다 연로그가 말한 메모리 절약 / 코드 가독성의 이점이 더 크다고 판단되면 계속해서 사용하면 되구요.
앞으로 어떤 것들이든 장단점이 공존할 것이고 각각의 장단점을 인지한 뒤 연로그의 기준으로 더 나은 것을 택하는 것을 계속 연습해가면 좋을 것 같아요. 개인적은 의견으로 지금은 메모리나 성능상의 이슈를 고민하기보단 각 미션에서 주로 관심을 두고 있는 것(이번 미션이라면 가독성과 메소드 분리 등)에 적절한 대안을 고르는 것도 좋을 것 같습니다.🙂
🙋♀️❓ 많이 사용하더라도 GC의 사용 영역 하에 두는게 좋을지, 많이 사용하니까 프로그램 종료 시까지 메모리가 할당된 채로 둘지에 대한 고민이었습니다. (이전에 책을 보며 정리했던 글: https://yeonyeon.tistory.com/113) 아직 이 질문에 대한 해결책은 찾지 못했지만 지금은 보류해두고 레벨업을 해가며 찾아보겠습니다.
🔻 컨트롤러도 테스트 코드를 작성해야 할까?
🙋♀️❓ 컨트롤러는 대부분 이미 생성한 Cars, Car에서 테스트를 마친 기능들을 가져다 쓰는 일이 대부분이었습니다. 컨트롤러에 대한 테스트를 만든다는 것은 '단위' 테스트라는 범위를 벗어나는게 아닐까? 라는 생각이 들어서 별도로 만들지 않았는데 혹시 잘못 이해한건 아닐지 정정 부탁드립니다!
🙍♂️👉 너무 좋은 고민인 것 같아요. Car Cars 단위테스트에서 모든 것이 테스트되기에 Controller는 무엇을 테스트해야하는가? 그 테스트가 단위테스트인가? 라는 의미로 받아들였는데 맞을까요? 맞다면 단위테스트 이외에 어떤 테스트들이 있는지도 찾아보면 좋을 것 같아요. 다른 테스트 중 컨트롤러에 적합한 테스트는 무엇인지도 공유해주세요🙂
🙋♀️❗ 네 맞습니다! 단위 테스트 말고도 통합 테스트, 인수 테스트 등이 있습니다. 이를 지원하는 많은 라이브러리나 프레임워크들이 존재하니까 다른 테스트 필요시 이를 활용하면 될 것 같습니다!
🔻 구조 변경 시 기존 테스트 코드를 리팩토링 해야하는가?
🙋♀️❓ 리팩토링을 진행하면서 Car에 int position, String name을 넣기보다 Position position, Name name 식으로 클래스를 따로 생성하게 되었습니다. 이 과정에서 name의 null 체크, 공백 체크 등 유효성 검증 로직이 Car가 아닌 Name으로 넘어가게 되었는데요. 이 경우 이름 검증에 대한 테스트 코드를 NameTest로 옮긴 후 리팩토링해야 할까요? 아니면 리팩토링 전에 이미 CarTest에서 테스트했던 부분이니 그냥 두는 편이신가요?
🙍♂️👉 역할이 이동했음에도 불구하고, CarTest에서 테스트했던 부분이니 넘어가는 이유가 있을까요? 어떤 기준으로 그냥 두어도 된다라고 판단한지 알고 싶어요!
🙋♀️❗ 테스트 코드를 삭제하면 안된다고 생각했는데 역할이 이동했으니 테스트 코드도 이동되어야 할 것 같습니다!
🔻 메소드명 짓는 방법
🙋♀️❓ 이름 짓기에 대해 많은.. 고민을 겪고 있습니다. 예를 들어 유효성 검증하는 메소드명에 대해서는 validateName으로 할지 isValidName으로 할지.. 이런 고민이요! 혹시 메소드 이름을 지을때 팁이나 참고할만한 자료/서적 같은걸 추천해주실 수 있으신가요?
🙍♂️👉 지금도 네이밍 충분히 잘하고 계신데요! 네이밍은 언제나 어려운 것 같아요😭 기본적인 룰은 인지하고 있되, 이 프로그램에서 의미하는 바를 명확히 표현하려는 연습을 계속하는 것 방법이 가장 중요한 것 같아요. 추가로 남의 코드나 잘 작성된 코드들 (프레임워크의 코드나, 자바의 코드) 등을 참고하고 주로 사용하는 용어들을 위주로 사용한다면 조금 더 빠르게 좋은 네이밍을 할 수 있지 않을까 생각합니다.
🔻 매직 넘버의 범위
🙋♀️❓ 의미 있는 값이라면 이름 짓기도 쉬운데 정말 '0'을 의미하는 값에 대해서도 상수 처리를 해야하는지 의문이 들어요. 예를 들어 자연수를 구하려고 할 때 number > 0를 체크하는 로직이 존재한다고 가정해보겠습니다. 여기서 0도 ZERO 같은 이름으로 상수 처리 해주어야 할까요?
🙍♂️👉 정말 0을 의미하는 것인데 상수처리를 해야한다고 생각하는 이유가 있을까요? 매직넘버라는 것에 대한 연로그만의 기준이 분명 필요한 것 같아요🙂 우테코에서 알려주는 것이나, 다양한 형태로 접하는 자료들은 일정수준 추상화된 룰을 제공하는 것이지, 정답을 제안하는건 아니라고 생각해요. 매직넘버를 상수화하는 것이 결국 무엇을 이루기 위해서 하는 행위인지, 그리고 매직넘버란 무엇인지에 대한 고민에 답이 나온다면 이 고민은 어느정도 해결될 것 같아요. 추가로 number > 0을 체크한다면 저는 0을 상수화하지 않고 메소드를 자연수인지 라는 형태로 변경할 것 같네요🙂
🙋♀️❗ 문자열이나 숫자를 그대로 쓰면 그건 다 하드 코딩이고 매직넘버라고 생각했는데 매직 넘버의 기준이 필요할 것 같다는 말씀을 듣고 깨닫게 되었습니다. 사용 목적이나 의미가 분명히 존재하는데 실질적인 값이 코드 내에 존재하는 것을 매직 넘버, 라고 정의해도 괜찮을까요?
🔻 다른 메소드에 의존적인(?) 테스트
🙋♀️❓ generateCarStatus라는 메소드를 테스트하기 위해 Car를 전진시키는 goOrStop 메소드를 이용합니다. 이는 goOrStop메소드가 정상 작동한다는 가정 하에 작성됩니다. generateCarStatus를 테스트하기 위해 작성한 테스트 코드인데 goOrStop에도 의존되는건 아닌가 의심이 드는데 어떻게 생각하시나요? 테스트 코드 작성을 위해 go()라는 메소드를 별도로 선언하는 방법도 생각해보았는데 테스트 코드만을 위한 메소드라는게 좋은 선택일지 잘 모르겠습니다.
🙍♂️👉 좋은 생각의 방향인 것 같아요. 서로 다른 역할을 하는 여러 객체들이 모여서 하나의 프로그램이 만들어질텐데요! 그렇다면 하나의 객체가 의존하는 다른 객체들의 역할을 모두 배제하고 해당 객체를 테스트할 수 있는 형태로 만들어야 할까요? 요 부분에 대해서는 앞으로도 고민해야하고, 할 일이 많을 것이기 때문에 답변을 드리기보단 미션마다 고민해보시는게 좋을 것 같아요🙂 단위테스트와 통합테스트 라는 키워드로 검색해보셔도 유의미한 글들을 많이 찾을 수 있을 것 같습니다.
🔻 객체의 상태를 이용해 문자열로 만들기
🙋♀️❓ 객체의 상태를 문자열로 만드는 로직은 어느 위치에 있어야할지 잘 모르겠습니다. 처음에는 출력을 위해 생성하는 문자열이니 OutputView에 있어야한다고 생각했습니다. 그 다음에는 OutputView는 출력만 할 수 있도록 String를 위한 StringUtils를 생성했습니다. 하지만 조금 더 생각해보니 객체의 상태를 이용해 특정 문자열을 생성하는 것이니 객체 내부에 존재해야 하는 것이 아닌가? 라는 생각이 들어서 돌고 돌아 객체로 다시 들아오게 되었습니다. toString()이라는 이름에서 generate~~ get~~ 이러한 이름으로 변경된 꼴이 되었네요.😂
🙋♀️❗ 다시 한번 생각을 해보고자.. 카일의 코멘트를 다시 읽었는데요 저는 getter 미생성에 집착해서 빙빙 돌았던 것 같아요. 중요한건 Model과 View의 분리이고, getter를 제한하는 목적은 객체에 메시지를 보내 스스로 상태에 대한 처리로직을 수행하는 것이라서 다시 리팩토링 진행했습니다😄
🙍♂️👉 이 부분도 너무 좋은 생각의 방향인 것 같습니다. Getter를 지양하자 라는 프로그래밍 요구사항의 의미를 명확하게 잘 파악해주신 것 같아요. 도메인과 View라는 레이어의 개념을 떠나서, 앞으로 주어지는 각 미션의 요구사항들이 왜 존재하는지에 대해 지금처럼 계속 고민해주시고 연로그만의 답변을 찾아, 그 원칙을 지킬 수 있으면 좋을 것 같습니다🙂
'Memo > 우테코 4기' 카테고리의 다른 글
[우테코] 로또 미션 1단계 학습 로그 (0) | 2022.02.28 |
---|---|
[우테코] 자동차 경주 2단계 학습 로그 (4) | 2022.02.23 |
[우테코] 우테코 4기 지원 후기 (10) | 2021.12.30 |
[우테코] 프리코스 3주차 회고 - 자판기 (0) | 2021.12.14 |
[우테코] 프리코스 2주차 회고 - 자동차 경주 게임 (0) | 2021.12.08 |