[우테코] 2주차 회고

2주차 미션

2주차 미션은 자동차 경주 게임이었다! 이 또한 주제만 보면 그렇게 어려워보이지는 않았지만 나에게는 큰 산이 있었다... 바로 테스트 코드 작성이라는 산이었다ㅠ 테스트 코드에 대한 개념만 알고 있고 직접 작성해 본 경험이 많지 않은지라 겁부터 먹었던 것 같다.😢 원래 포함되어 있단 test/java/study 내 코드들을 살펴보기도 하고 따로 테스트 코드 작성법에 대해 공부도 많이 해야 했었다.

 

 

기능 목록

1주차보다 꼼꼼히 작성하려고 노력하였다. 하지만 뭔가 마음에 들지 않았는데... 이유를 다시 생각해보니 나는 클래스와 함수를 먼저 다 짜놓고 난 후에 기능 목록을 작성하려고 하였는데, 이 방법이 오히려 어려웠던 것 같다. 차라리 정리되지 않은 기능들을 먼저 쭉 적어보고 그 후에 함수나 클래스로 나누는 것이 좋을 것 같다. 

 

 

PR 리뷰

나는 내 코드와 일주일 간 씨름하기 때문에 익숙해지기 마련이다. 그렇기에 미처 보지 못하고 넘어가는 부분들이 많은 것 같다😢 특히 다른 사람들에게도 쉽게 읽히고 이해되는 코드를 작성해야겠다는 생각이 크게 들었다.

 

내가 받은 리뷰들이다.

 

따로 copy해서 넘겨주는 건 값이 변하지 못하도록 하기 위해서인가요??

comma가 아니라 다른 값으로 변경되면, 함수명과 split부분을 수정해야 할 것 같습니다. ㅎㅎ 그런 경우를 대비해서 좀 더 일반화된 이름이 좋을 것 같아요

set자료구조나 아니면 stream API에 distinct() 메서드를 사용하면 더 깔끔해질 것 같아요 ㅎㅎ

여기 enum 클래스는 밖으로 빼도 보기 좋을 것 같습니다 ㅎㅎ

validateCarNameLength에 다른 함수와 같이 list로 받아서 하는게 좀 더 통일감 있어 보일 것 같습니다 ㅎㅎ

혹시 GameStatus는 따로 enum 클래스로 구현하지 않고 내부에 private enum으로 구현하신 이유가 있을까요?
그리구 이번 미션은 횟수를 알 수 없는 저번 미션과 달리 정해진 횟수만큼 게임을 진행하는데 GameStatus를 따로 구현하신 이유가 궁금해요!🤔

다른 분들의 코드를 보면서 알게 되었는데 같은지 확인하는 메서드는 Comparable 인터페이스의 compareTo를 사용하는 것들을 많이 추천하시더라구요!

혹시 MoveManger을 따로 구현하신 이유가 있으실까요?
움직일지 말지 판단하는 것은 Car가 직접 판단해도 괜찮지 않을까요?
코드의 이유가 궁금해요🤔

공백을 예외처리가 아닌 제거하신 이유가 궁금합니다!
"po bi"나 " nana", "a bcc"와 같은 경우도 모두 정상적인 입력으로 처리해주시는 건가요?

공백을 여러 곳에서 사용한다면 따로 enum 클래스를 생성하여 사용하는 것도 좋지 않을까 건의드려 봅니다 ㅎㅎ

2와 5 같은 매직넘버는 상수 처리나 enum 클래스로 매직 넘버만 관리해도 괜찮을 것 같아요😁
또는, 해당 부분을 "%d"와 같이 빈칸을 두어서 포매팅을 하는 방법도 가능할 것 같아요!

게임이다보니 누구나 사용할 수 있게끔 int 보다는 "~~ 범위를 초과하는"과 같이 작성하는게 더 좋지 않을까..! 생각해봅니다 ㅎㅎ

RandomNumberValidator에서 0과 9를 검증을 하는데 상수처리를 하여 0과 9사이의 값만을 호출하게끔 구현하면 따로 검증할 이유가 있을까요? 궁금해서 질문 남깁니다!!

위에 상수처리를 하였는데 상수를 활용해도 괜찮을 것 같아요😃

이동 유무를 받아오는 해당기능을 Controller에서 파라미터로 입력받아오는 것은 좋은 방법은 아닐것 같아요. 객체나 서비스에서 사용하시는게 어떨까요..?

불변 리스트로 생성할 수 있는 건 처음보네요.. 변경되지 않을 객체를 제한할 수 있다면 이렇게 제한하는게 좋은 것 같아요

해당 enum은 따로 빼두시는 게 좋을 것 같아요. 클래스 내부에 있는 줄 모르고 파일명을 찾고있었어요

static으로 선언되어 있어 어디서든 Randoms.pickNumberInRange 메소드를 가져올 수 있는데 굳이 클래스로 한번 다시 감싸서 사용할 필요는 없을 것 같습니다!

저는 반대로, 이런 접근이 조금만 더 발전하면 효과적으로 Randoms 클래스와 비즈니스 로직 간의 결합도를 낮출 수 있을 것이라 생각해요.수를 생성하는 기능을 정의하는 인터페이스를 만들어 두고, 해당 인터페이스를 구현하도록 만들면 테스트에서는 "4만 생성해 주는 수 생성 클래스" 와 같은 다른 클래스로 갈아 끼워 테스트를 훨씬 쉽게 진행할 수 있거든요!

String 클래스에 특정 문자열을 반복해주는 repeat 함수도 있어 쉽게 바꿀 수도 있을 것 같아요

객체를 싱글톤으로 사용하는 방법은 좋은 방법 같아요. 저도 고려해봐야겠네요

난수 생성을 테스트하는 방향이 좋을 것 같아요. 저도 테스트는 거의 안써봐서 Randoms 테스트 하는 메서드를 그대로 사용했는데 공부를 해야할 것같습니다 ㅎㅎ..

람다식이랑 메서드 참조 능숙하게 쓰시는 거랑 맵으로 게임 상태를 관리하는 게 인상 깊어요
코드가 논리적이어서 이해하기 수월했습니다.
개인적으로는 progress, play랑 race,moveAllCarList가 코드 상에서 같이 배치되어 있었으면 조금 더 직관적으로 느껴질 것 같아요
많이 배웠습니다!

움직일지의 여부 boolean을 인자로 받아오는 것이 굉장히 어색한 것 같습니다.해당 메서드에 boolean을 넣어주는 객체는 사실 해당 메서드를 호출할지 말지의 정보를 알고있는 것과 마찬가지니까요.
boolean을 외부에서 사용해서 move() 메서드를 호출하거나, boolean이 아닌, 움직일지 말지를 확인할 때 사용할 특정 정보를 전달해 주는 편이 훨씬 좋아 보입니다!

현재의 상태를 "캡쳐를 뜨는 것 처럼" 복사해서 넘겨주는 것이 굉장히 인상깊어요!!
이 덕분에 외부에서의 값 변경으로 인한 사이드 이펙트를 많이 예방할 수 있을 것 같습니다!!

isSame() 메서드가 어떻게 사용되나 했더니, 여기서 사용되는군요!!
그런데, isSame()이라는 네이밍은 Car 객체들간의 동일성 비교를 하는 메서드처럼 예상이 됩니다!
isLocatedAt()과 같은, 더 목적을 잘 나타낼 수 있는 네이밍을 고민해 보시면 좋을 것 같아요

여기선 List<String> 객체를 List<Car> 객체로 바꿔주고 있는데, CarList 클래스 내부에선 다시 검증을 위해 List<String>으로 변경한 뒤 검증 로직을 호출하고 있어요.차라리 해당 클래스에서 검증 로직을 호출 후 CarList 객체를 생성하는 것은 어떨까요?

또는, 해당 부분을 "%d"와 같이 빈칸을 두어서 포매팅을 하는 방법도 가능할 것 같아요!

Car 객체를 테스트 내에서 매번 사용할 거라면, 차라리 static 변수로 미리 생성해 둔 후, 가져와 사용하는 것이 훨씬 깔끔할 것 같아요

CarMoveManager 와 CarRegister 의 초기화를 다르게 하신 이유가 따로 있을까요? 어떤 고민을 하시다 이렇게 구성하신건지 궁금해서 코멘트 남깁니다!

Map 과 Supplier 를 활용해서 게임 상태를 관리하는게 인상깊었어요!
대신 처음 컨트롤러 로직을 봤을 때 조금 이해하기 힘들었는데, 아마 로직의 리턴값들이 꼬리를 물고 이어져서,,? 조금 헤맸던 것 같아요.
문장이 너무 길면 이해하기 힘들듯이 로직도 한번쯤 끊어가면 직관적으로 이해하기 조금 더 쉬워지지 않을까 라는 생각이 들었습니다!

개인적으로 자동차의 이름 길이 5자 이내는 Cars 에서 검증이 아니라 Car 에서 해야한다고 생각해요!
만약 Car 에서 이를 검증하면, Car 객체는 무조건 이름이 5자 이내인 객체이니 다른곳에서도 활용할 수 있을 것 같아요.
지금 상황에서 Car 객체를 재활용? 한다면 이름에 대한 검증이 없는 Car 객체를 사용했으니 이에 대한 검증을 사용하는 곳에서 다시 해줘야 할 것 같아요.
저는 그래서 항상 데이터를 가지고 있는 객체에서 검증을 진행하려고 합니다..!

랜덤값을 이렇게 도메인 안에서 생성하면 테스트하기가 어려워지더라구요
저는 그래서 랜덤값을 최대한 외부에서 파라미터로 넣어주려 합니다..!

 

대부분 비슷한 부분에서 수정할 점들을 말씀해주셨다!

• splitByComma 메서드
• enum 클래스 작성
• 컨트롤러 클래스의 로직
• 검증 메서드의 위치 변경
• 출력 포매팅

등등... 다시 정리해서 고민해봐야 할 것 같다! 우테코에서 가장 좋은 부분이 이 PR 리뷰가 되었다❤️ 다른 분들의 코드를 보는 것도 많은 공부가 되고 이렇게 리뷰를 모아 어떤 점을 발전시켜 나가야할지 분석하는 것도 즐겁기 때문이다. 

 

 

 

스스로에게 하는 피드백

1. 다른 메서드로 쉽게 풀어낼 방법이 있는지 확인한 후 작성하자!
2. enum 클래스는 따로 만들어 작성하자!
3. 클래스명과 함수명을 더 이해하기 쉽게 작성하자!
4. 코드를 통일성있고 간결하게 작성하자!
5. 로직이 꼬리를 물지 않도록 작성하자!
6. 내가 왜 이렇게 코드를 작성했는지 명확히 설명할 수 있도록 하자!

 

 

https://github.com/woowacourse-precourse/java-racingcar-6/pull/1585

[자동차 경주] 임규리 2주차 미션 제출합니다. by IM-GYURI · Pull Request #1585 · woowacourse-precourse/java-rac