각종 후기/우아한테크코스

[우아한 테크코스 3기] LEVEL 1 회고 - 자동차 경주 게임의 1차 피드백을 받아보다 (5일차)

제이온 (Jayon) 2021. 2. 6.

안녕하세요? 제이온입니다.

 

어제 코드 리뷰를 신청했는데, 놀랍게도 리뷰어 님이 약 2시반 30분 만에 답변을 주셨습니다. 저는 원래 주말에 보려고 하였으나, 첫 피드백인만큼 설레는 마음으로 바로 확인하였습니다 ㅎㅎ

 

오늘은 피드백 내용과 제가 수정한 부분을 말씀드리겠습니다.

 

 

1차 피드백

 

 

이런 방식으로 제가 코드 리뷰를 요청하면, 리뷰어 분이 피드백을 남겨주시는 방식으로 온라인 코드 리뷰가 진행됩니다. 귀한 시간 내 주신 것에 다시 한 번 감사드립니다!!

 

먼저, 첫 번째 수정 사항은 'String...'을 'String ...'으로 수정하는 것입니다. 이것은 String args에 해당하는 것인데, String과 ...을 띄는 것이 좋다는 것을 처음 알았습니다. 혹시 이 'String ...'이 궁금하신 분은 이곳을 참고하시길 바랍니다.

 

 

 

 

두 번째 수정 사항은 상수 처리입니다. 제가 검토를 하다가 이런 중요한 사항을 빼 먹었군요. 바로 고쳐두었습니다.

 

 

 

 

세 번째 수정 사항은 메소드 기능을 통합하라는 것입니다. 저는 처음에 GasStation 클래스를 통해 이 메소드를 호출하여 연료를 충전하고, 차를 달리게 하는 방식으로 코드를 작성하였는데, GasStation 클래스가 삭제되었으니 fullInFuel() 메소드도 삭제하는 편이 좋겠다고 판단하였습니다.

 

 

 

 

그래서 인자로 충전할 연료를 받고, 연료를 충전하고 그 양에 따라 달리는 것으로 수정하였습니다. 이미 피드백 내고 나서 생각하는건데 메소드 이름을 fillInFulAndRun() 으로 바꾸는 편이 더 좋은 것 같기도 합니다. 이것은 2차 피드백 이후에 고민해 보겠습니다.

 

 

 

 

네 번째 수정 사항은 README 소제목 변경인데, 이제부터 README에 구현할 기능 목록 작성보다는 요구사항 정의라고 쓰겠습니다 ㅎㅎ

 

 

 

 

다섯 번째 수정 사항은 내장 클래스 사용입니다. 이 부분은 제가 프리코스때 주어진 RandomUtils를 반드시 써야한다고 생각하여 이 클래스의 nextInt() 메소드를 사용한 것인데, 다시 생각해 보니 굳이 의미가 없었습니다. 그래서 대신 Random 클래스를 사용하여 난수를 만들어냈습니다. 이것도 피드백 내고 생각해 보니 RandUtils 패키지를 아예 삭제해야겠습니다 ㅋㅋㅋ

 

 

 

 

여섯 번째 수정 사항은 줄바꿈 문자입니다. OS에 따라 이 줄바꿈 문자가 다르기때문에 '\n'으로 처리하지 말고 System.lineSeparator()를 사용하라는 것이죠. 이것은 완전 새롭게 알게된 것이라 흥미로웠고, 다음부터는 주의해야겠습니다.

 

 

 

 

일곱 번째 수정 사항은 변수명 변경입니다. 자동차 경주 게임에서 연료는 랜덤값으로 충전되므로 단순히 변수명을 fuel으로 하기보다는 randomFuel이 낫다고 판단됩니다.

 

 

 

 

여덟 번째 수정 사항은 파라미터 삭제입니다. fuel을 생각해 보니, 이미 인스턴스 필드로 갖고 있었는데도 의미 없이 fuel을 또 파라미터로 주입받았네요. 바로 지웠습니다.

.

 

 

 

아홉 번째 수정 사항은 테스트 메소드명 변경입니다. 어.. 근데 이건 우아한테크코스 강의자료에도 테스트 메소드명이 한글로 나와있어서 그렇게 써도 되는 줄 알았습니다. 그리고 메소드명을 영어로 하되, 의미 전달이 잘 되면 DisplayName 어노테이션은 필요 없을 것 같다고 판단하여 메소드명만 영어로 수정하였습니다.

 

 

 

 

마지막 수정 사항은 객체지향적인 코드로의 변경입니다. 이 부분이 제일 고민이 많았는데, Participants 클래스에서 우승자를 반환하는 방식으로 수정하면 괜찮지 않을까 생각하였습니다.

 

 

 

 

Participants 클래스는 멤버 변수로 List<Car>를 갖고 있기때문에 바로 이 리스트를 이용하여 우승자를 가려낼 수 있습니다. 다만, maxPosition을 구할 때 orElse() 메소드로 초기값을 정해주어야하는데, 이를 상수로 하게 되면 멤버 변수에 상수를 추가해야합니다. 그런데 Participants 클래스가 DEFAULT_MAX_POSITION 상수를 갖는 것이 바람직하지 않는 것 같아서, 굳이 상수 처리를 안 하고 0으로 써도 되는지 궁금하여 이 부분은 리뷰어 님께 질문하였습니다.

 

 

 

 

정리

인생 처음으로 코드 리뷰라는 것을 받아 보았습니다. 수정 사항이 10가지나 있지만, 그래도 중요한 로직이 잘못된 것은 아니라서 꽤 기분이 좋았습니다. 어젯밤에 피드백을 열심히 반영하여 오늘 오전에 다시 코드 리뷰를 요청드렸고, 2차 피드백이 오는대로 바로 수정해 봐야겠습니다~

 

위 미션의 코드가 궁금하신 분은 이곳을 참고하시길 바랍니다.

댓글

추천 글