Step4 - 자동차 경주(우승자) - 피드백 반영 (11일차)

안녕하세요 멘토님!!
기존에 pull request가 닫히지 않아서 그대로 여기에 작성합니다.
제가 수정한 커밋들이 지금은 보이지 않는데, github설명을 보니 그대로 이어서 pull request를 작성하면 되는 것 같아
그대로 연결하여 진행하도록 하겠습니다.

네 맞습니다 😄
이 후 미션 진행하실 때도 Review Request 되면 같은 PR에 그대로 작업해주시면 됩니다 😄

말씀해주신 피드백 너무 감사합니다.
모든 피드백이 이해가 갔지만 validInputConfirm()을 객체가 생성할 때 검증을 하라는 것이 처음에는 저에게 와 닿지 않았습니다.
그럴거면 차라리 validInputConfirm() 부분을 input 받을 때 바로 검증하는 것이 더 좋지 않을까 라는 생각이 들었기 때문입니다.

조금 더 생각해보니 사용자로 부터 input값을 받는 메소드를 제가 만든 코드가 아닌 다른 사람의 것을 사용하는 경우에
해당 메소드에는 input값을 받기만 하고 validInputCheck를 진행하지 않는다면 코드 검증을 할 수 있는 방법이 없어지기 때문에 객체를 생성하는 시점에서 input값을 확인하는 것이 더 좋다고 말씀하신 것 같다. 라고 파악하면 될까요??

네 맞습니다 아주 정확하게 이해하셨어요 😄
validate체크를 해서 통과가 되면 객체가 초기화 되도록 강제한다면
이후 객체가 가진 값에 대해서 많은 부분 신뢰할 수 있게 됩니다 😄
잘 이해해 주셨습니다 🙇

테스트를 작성하는게 뭔가 너무 어색합니다.ㅜㅜ
테스트는 최대한 간단하게 짜라고 하셨는데, 작성을 하다 보니 어느새 본래 코드와 다를바가 없어지고 있어서 당황스러운 상태입니다.
조금 더 살펴보고 또 추가해 보도록 하겠습니다.
감사합니다!

테스트를 최대한 간단하게 작성하라의 의미를 다시 잘 생각해보면
테스트가 최대한 간단하게 작성이 되어야 한다는 의미이기도 합니다
그러려면 어떻게 해야 할까요 ?? 😅
테스트 대상인 객체들이 최대한 테스트가 간단하게 작성 될 수 있도록
응집되고 메소드 단위도 적절해야 하며 구조도 간결해야 함을 의미합니다 😄

개인적인 의견을 하나 더 드리자면
테스트 작성이 어려운 객체일 경우 그 때가 리팩토링의 시점이라고 생각 합니다 😄

잘 해주고 계시니 너무 좌절하지 마시고
꾸준히 포기만 안하시고 잘 따라와 주시면 됩니다 🙇
화이팅 입니다 🔥

---

안녕하세요 재영님 🙇

지난 번 3단계 리팩토링에 이어 추가적으로 4단계 요청사항 구현 잘 해주셨습니다 😄

그래도 이전 보단 많이 구조가 개선이 되었는데,
4단계 요구사항에서 조금만 더 다듬어지면 좋을 것 같아서
추가로 의견 더 드렸습니다 🙇

다소 지치시겠지만, 한번 만 더 확인해주시고
개선 검토 해주시면 감사하겠습니다 🙇

화이팅 입니다 💪

 

💯💯💯💯💯
이렇게 함으로써 nameOfCar, numOfCycle 값이 올바르지 않은 경우
경기 자체를 시작할 수 없게 됐네요 👍

---

else 예약어를 쓰지 않는다.

• 힌트: if 조건절에서 값을 return하는 방식으로 구현하면 else를 사용하지 않아도 된다.
• else를 쓰지 말라고 하니 switch/case로 구현하는 경우가 있는데 switch/case도 허용하지 않는다.

이 부분 한번 적용시켜 보시는건 어떨까요?? 😄
그리고 추가적으로 하나 더 의견을 드리자면 우승자를 구하는 로직이 ResultView에 있습니다 😄
만약 콘솔 출력이 아닌 우승자 정보를 어딘가에 저장한다거나 혹은 html, json 형태로 전달해야 한다면
자동차 게임과는 상관 없이 계속 이 ResultView를 수정하고 사용해야 합니다
(물론 이번 요구사항은 아닙니다만, 확장성 관점에서 의견 드립니다 😄)

그리고 이 ResultView가 없이는 자동차 게임의 우승자를 알 수도 없구요 😭
이 부분 한번 만 더 개선 검토 해주시면 좋을 것 같습니다 😄 🙇

 

src/main/java/step4/ResultView.java

if (carPosition > maxCarPosition) {
	maxCarName = carName;
	maxCarPosition = carPosition;
} else if (carPosition == maxCarPosition) {

---

int[] carPositions = {1,2,3,4,3,4};
String[] carNames = {"a","b","c","d","e","f"};

for (int i = 0; i < carPositions.length; i++) {

 

고민 하신 부분이 이 부분인 것 같은데 😄

테스트 코드와 원래 코드가 자꾸 닮아간다면 이건 분명
구조적으로 리팩토링을 해야하는 신호이기도 합니다 😄

테스트는 원래 코드의 부분 부분이 잘 동작하는지를 보기 위함입니다 😄
반대로 생각해보면 원래 코드가 부분 부분으로 나뉘어져 있지 않다는 의미 이기도 합니다 🙇

개선 검토 해주시면 감사하겠습니다 😄

 

 

 

피드백 반영 1 

else 예약어를 쓰지 않는다. 

The ThoughtWorks Anthology 챕터중 객체 지향 생활 체조 부분에서 나온다고 한다.

else 를 쓰게 되면 가독성이 떨어진다고 한다.
힌트에 있는 것 처럼 return을 사용해서 else를 삭제하고 if문을 추가하였습니다.

indent가 맞춰지면서 가독성이 좋아졌다는 점은 있지만,
어떤 점이 더 좋아진 건지는 정확히 모르겠습니다.

if (carPosition > maxCarPosition) {
	maxCarName = carName;
	maxCarPosition = carPosition;
} else if (carPosition == maxCarPosition) {

private void findMaxPositionCarName(int carPosition, String carName) {
        if (carPosition > maxCarPosition) {
            maxCarName = carName;
            maxCarPosition = carPosition;
            return;
        }
        if (carPosition == maxCarPosition) {
            maxCarName += ", "+ carName;
            return;
        }
    }

 

피드백 반영2

그리고 추가적으로 하나 더 의견을 드리자면 우승자를 구하는 로직이 ResultView에 있습니다 😄
만약 콘솔 출력이 아닌 우승자 정보를 어딘가에 저장한다거나 혹은 html, json 형태로 전달해야 한다면
자동차 게임과는 상관 없이 계속 이 ResultView를 수정하고 사용해야 합니다
(물론 이번 요구사항은 아닙니다만, 확장성 관점에서 의견 드립니다 😄)

그리고 이 ResultView가 없이는 자동차 게임의 우승자를 알 수도 없구요 😭
이 부분 한번 만 더 개선 검토 해주시면 좋을 것 같습니다 😄 🙇

사실 저도 많이 고민했던 부분입니다.
우승자를 구하는 로직이 race에 들어있는게 구조적으로 더 아름다워 보이긴 했습니다.
그러나 우승자를 구하는 로직까지 race에 들어가면 race 클래스의 덩어리가 너무 커지는 것 같아 무서웠습니다.
그렇다면 그 다음으로 이 로직이 들어가도 어색하지 않을 공간은 어디일까라고 생각하여서 넣은 곳이 resultView 단입니다.
멘토님이 보기에도 제 race 클래스의 무게감이 너무 커 보이지는 않는가요??

그래서 궁금한 점이 있습니다.
지금 제 구조를 봤을때 문제가 있어 보이지는 않은가요??
race 코드를 다시 분리를 시키는게 옳을까요??
멘토님은 레이싱 게임 프로젝트를 하실 때 어떻게 코드를 작성하셨는지 궁금합니다!

 

음... 테스트 코드를 약간 변경하긴 했는데,, 제가 생각했을때는 여기서 더 작게 나누는 방법이 떠오르질 않습니다.
이정도는 괜찮은 것 같은데, 멘토님이 보기에는 어떤가요??