🪐우아한테크코스 6기 프리코스 2주차 전체 피드백, PR 리뷰 내용 정리🪐

---

전체 피드백

1. README.md를 상세히 작성한다. (3점)

최대한 상세히 작성하기는 하였지만, 불필요한 내용들이 많았던 것 같다. 특히 이번주차부터는 해당 프로젝트가 어떤 프로젝트인지를 더 나타내주어야 좋을 것 같다.

2. 기능 목록을 재검토한다. (2점)

리드미를 상세하게 적으려고 하다가 보니, 모든 메소드들의 이름을 함께 적어버렸다. 하지만 이렇게 계속해서 변경될 수 있는 부분들은 작성하는 걸 지양하라고 한다. 그러므로 구현할 기능 목록에만 더 집중을 하면 좋을 것 같다.

추가적으로 정상적인 상황 말고, 예외적인 상황도 중요하기에 이러한 부분도 정리해서 작성해야겠다.

3. 기능 목록을 업데이트한다. (4점)

리드미도 분명 변할 수 있기 때문에, 여러 번 수정을 하면서 업데이트를 진행했었다. 물론 불필요하고 반복적인 업데이트는 지양하는 것이 좋겠다.

4. 값을 하드 코딩하지 않는다. (2점)

1점을 줘야하지만, 이번에는 입출력 메세지와 에러 메세지는 상수로 만들었으므로 2점을 주었다.

정수들을 매직넘버로 사용했기 때문에, 이 부분은 꼭 수정이 필요할 것 같다.

5. 구현 순서도 코딩 컨벤션이다. (2점)

잘 몰랐던 부분이다.

1. 상수
2. 인스턴스 (멤버) 변수
3. 생성자
4. 메소드

순으로 코드를 작성해야한다고 한다. 잘 기억해두자.

6. 변수 이름에 자료형을 사용하지 않는다. (1점)

지키지 못한 부분이다. 차 이름이 저장된 배열을 carNamesArray와 같이 사용해버렸다.

7. 한 함수가 한 가지 기능만 담당하게 한다. (4점)

함수는 기능을 최대한 나누려고 노력하기는 했다.

다만 controller 부분에서의 분리가 아쉬우니, 이 부분 신경써서 구현해야겠다.

8. 함수가 한 가지 기능을 하는지 확인하는 기준을 세운다. (3점)

함수를 적절하게 나누는 본인의 기준이 중요하다는 말인 것 같다.

이는 계속해서 반복하며 감을 잡는 것이 중요할 것 같다.

9. 테스트를 작성하는 이유에 대해 본인의 경험을 토대로 정리해본다. (3점)

테스트 작성이 단지 기능을 점검하는 이유만 있지 않다고 한다. 본인의 코드에 대해 빠르게 피드백을 받거나, 학습 도구로도 사용할 수 있으니, 이러한 부분도 생각해보면서 테스트의 유용함을 알아볼 필요가 있다.

10. 처음부터 큰 단위의 테스트를 만들지 않는다. (3점)

오히려 이번에는 너무 작게 메소드 하나하나 씩만 만들어서 아쉬웠던 것 같다. 그래서 피드백으로도 기능을 검증하는 것이 아니라, 메소드가 잘 짜여졌는지만 확인하는 느낌이라고 했다.

이번에는 기능 면으로 잘 나누어서 테스트 코드를 작성해보아야겠다.

---

PR 리뷰

1. 매직 넘버를 사용

• 문자열은 잘 처리했지만, 정수는 그대로 매직 넘버를 사용하였음
• 심지어 상수 ONE_STEP을 만들었음에도, 사용을 하지 않음

2. 자바 컨벤션을 지키지 못한 부분이 있음

• 함수 인자를 여러 개 넣을 때, 콤마 뒤에 공백 필요
• 제네릭스 타입 선언 시에도 1번과 동일, 콤마 뒤에 공백 필요

3. 변수명에 자료형이 들어가 있음

• carNamesArray와 같은 네이밍 X

4. 리드미에 함수명을 쓰는 것을 지양

• 코드가 바뀔 때마다 매번 변경할 수는 없으니, 다음부터는 모든 함수를 쓰는 것보다 전체적인 기능만 적는 게 나아 보임

5. Controller 클래스 내부에서만 사용하는 메소드들을, public으로 선언함

• 시간이 부족하여 제대로 체크하지 못한 점, 다음부터는 접근 제어자를 잘 확인해야 할 듯함

6. 에러 메세지를 별도로 출력하기보다는, 에러 메세지 값을 전달

• IllegalArgumentException()에 에러 메세지 값을 전달하는 방식으로 수정

기존

public static void validateIsNotNull(String inputString){
        if (inputString == null
                || replaceCommaToBlank(inputString).isBlank()) {
            ErrorMessage.isNotNullError();

7. when then을 꼭 구분하지 않아도 됨. 제공된 테스트를 참고.

• 추가적으로 던지는 메세지가 적절한지도 테스트

8. 시도하는 횟수인 tryNumber은 중요한 변수라 따로 관리해도 좋아 보임

9. Utils로 세세하게 나눈 것은 좋으나, 조금 불필요한 메소드 생성은 지양해도 좋아 보임

10. 함수 네이밍 시 축약 하지 않도록, 이름을 통해 의도를 드러내도록

• 차를 이동시키는 move의 경우에도 들어오는 랜덤 값에 의해 움직이거나 움직이지 않을 수 있기 때문에, moveOrStay같은 네이밍이 적절

11. Enum 타입을 활용하는 것 추천

• 3주차 미션부터는 Enum 타입을 활용해서, Input 클래스에서 입력에 관한 출력까지 모두 하도록 적용

12. Controller 내부에서 기능을 더 잘 나누는 것이 좋아 보임

• 예를 들어 입력을 받아오는 getCars라는 메소드에서

1. Input 메세지 출력 후 Input
2. 유효성 검사
3. 배열로 받아옴

까지 진행한다면 가독성이 훨씬 좋아질 것

13. static을 많이 사용했는데, 이는 객체지향적으로 좋지 않음

• 지금까지는 같은 클래스에서 메소드를 호출하려면 static을 붙여야한다고 생각했는데, 객체를 생성해서 호출하면 간단히 해결할 수 있음
• 3주차부터는 꼭 고쳐야 할 듯

14. 테스트의 목적을 다시 생각. 함수가 잘 만들어졌는지 보다는, 구현한 클래스의 기능 검증이 중요한 것

• 그러므로 테스트 코드를 너무 많이 만들기 보다는, 정한 기능마다 테스트 코드를 잘 작성하는 것이 좋아 보임

15. Utils를 더 쪼개는 것도 괜찮아 보임

• stringUtils, randomUtils 등으로 세분화 해보기

16. 에러 메세지에서 더 의도를 드러내도록 네이밍

• IS_NOT_NULL_ERROR_MESSAGE ⇒ NULL_ERROR_MESSAGE
• boolean 타입은 is~ 나 has~로 네이밍

17. 어차피 우승자 이름만 출력해주면 되기에, 단독 우승과 공동 우승을 출력할 때 나눌 필요가 X

• ArrayList의 크기가 1이면 String.join(", ", jointWinners)의 결과값이 soloWinner.get(0)와 동일하기 때문에 하나의 메소드에서 처리할 수 있음

18. 에러를 던질 때, 에러 메세지도 함께 던져주기

throw new IllegalArgumentException(ErrorMessage.separatorError());

같은 방식으로 출력할 수 있도록 해보기

19. HashMap을 사용한 것은 좋은 아이디어이나, 순서가 보장 X

• LinkedHashMap을 사용하면 순서가 보장됨

20. 불변 객체 생성 시, record 사용도 고려

• record 클래스를 사용하면, 더욱 편리하게 관리가 가능

21. 함수 네이밍 시, 동사가 맨 앞에 가도록

예) 문자열을 정수형으로 바꾸어 주는 메소드

• 기존 : StringToInt
• 변경 : ConvertStringToInt