[2단계 - 자동차 경주 리팩터링] 야미(이다인) 미션 제출합니다.

[feb-dain]

코드 리뷰 1단계

리뷰 반영

constant

commit (90ca704)

• 정규표현식도 상수로 관리
• 에러 메세지 : 상수 재사용

util

commit (2e91f47)

• splitAndTrimString으로 메서드명 변경

commit (0c9d0cc)

• 10도 상수 처리

validator

commit (0041d1a)

• isNaN 대신 정규 표현식 사용

App.js

commit (9a1d508)

• index.js가 아닌 App.js에서 new CarGame() 사용

domain

commit (95a1210)

• getter, setter 삭제 + carGame 내부에서만 carStatus 사용
  ( 객체 대신 new Map을 사용하는 방식으로 바꿨는데,
  reduce 보다는 forEach가 가독성이나 속도면에서 좋을 것 같아 reduce 대신 forEach를 사용했습니다! )
• constructor 삭제(default value 활용)

commit (167440c)

• 상수명 변경 : max -> farthestDistance

질문

Q❓ 페어프로그래밍을 같이 한 짝이 받은 피드백
("4를 넘으면 한 칸을 움직이고 8을 넘으면 두 칸을 움직이게 한다면 어떻게 코드를 수정해야 할까요?...")을 보고
MovementIndicator는 잘 변하는 기능이라고 생각해 아래와 같이 코드를 변경했습니다.
유연하게 수정 가능한 코드로 바꾸고자 한 제 의도가 잘 드러났는지 궁금합니다!

commit (0ec5a65)

const isMoving = (randomNumber) => {
  return Boolean(randomNumber >= FORWARD_CONDITION_NUMBER);
};

👇

const movingDistance = (randomNumber) => {
  return randomNumber >= FORWARD_CONDITION_NUMBER ? FORWARD_DISTANCE : 0;
};

---

Q ❓기능과 매치가 안 되거나 이해하기 어려운 변수명, 메서드명이 있는지 궁금합니다!
더 좋은 이름이 있다면 알려주시면 감사하겠습니다!
Q ❓폴더명과 폴더를 나눈 방식이 코드를 이해하는 데 도움이 되었는지 궁금합니다!
Q ❓기능을 나눈 방식이 괜찮은지 궁금합니다!
Q ❓파일, 폴더명 관련 컨벤션 규칙도 있는지 궁금합니다!

코드 리뷰 2단계

• 폴더명 : 대문자 -> 소문자로 모두 변경, UI 폴더 -> view 폴더로 이름 변경 (c53ebdb)
• 쓸모없는 코드 삭제 (f1895ff, d50b4df)
• 재사용성 높이기 위해 랜덤 숫자 생성기의 코드 변경 (4f47c29)
• 더 정확한 메서드명 사용 (65b95d1)

🎯 리팩터링 요구 사항

• 도메인 로직은 domain/ 하위로, UI 관련 로직은 view/ 하위에서 관리한다.
• domain/ 하위의 모듈은 view/ 하위의 모듈을 의존하지 않아야 한다.
• 도메인 로직 내에서 테스트하기 어려운 부분을 분리하고, 테스트 가능한 부분에 대해서만 단위 테스트를 작성한다.
  • 도메인 CarGame에서 car status를 prefix로 사용하고 있기에 테스트하기 어렵다고 판단
    => CarGame 관련 테스트 코드를 작성하지 않았습니다.
• 테스트 코드에서 jest.fn()을 사용하지 않는다.

리뷰 반영

• errorCatcher : currying 사용

currying을 사용해보려고 했으나 처음이라 이렇게 하는 게 맞는지 잘 모르겠네요 😂
코멘트로 질문 드렸는데 답변 부탁드립니다!

[InSeong-So]

안녕하세요, 야미😊 리팩토링 고생하셨네요.
1차 미션에서 봤던 함수가 좀 바뀌어서 놀랐는데, 이유를 적어주셔서 재밌게 읽었습니다.

movingDistance에 대한 의도

• 그런 의도였군요! 4를 넘으면 한 칸을 움직이고 8을 넘으면 두 칸을 움직이게 한다면 어떻게 코드를 수정해야 할까요?...가 저를 좀 더 헷갈리게 만든 것 같아요. 현재 설명해준 로직에서는 단순 삼항식으로 전진하거나/하지 않거나인데, 확장하게 된다면 조건마다 전진에 대한 분기를 태우는 걸까요?👀 그렇게 되면 유지보수가 오히려 어려워질 것 같다고 생각했는데, 의도가 잘 전달된건지는 모르겠네요... 이 부분 설명 다시 한 번 부탁드려요🙇‍♂️

Q ❓기능과 매치가 안 되거나 이해하기 어려운 변수명, 메서드명이 있는지 궁금합니다!

• 야미가 약간... 일반적이지 않은 변수/함수명을 쓰는 느낌이 들었어요. 가령 farthestDistance, MovementIndicator... 전자는 maxForward가, 후자는 해당 파일을 제거하고 movingDistance를 carGame에 메서드로 선언하면 어떨까... 그렇다면 좀 더 전달이 잘 되지 않을까 생각했습니다.

Q ❓폴더명과 폴더를 나눈 방식이 코드를 이해하는 데 도움이 되었는지 궁금합니다!

• 괜찮았습니다! domain, view 등은 의도한대로 전달되었어요. 다만 폴더명/파일명이 동일하다면 index 파일을 두고 named export를 진행해주면 어떨까 싶네요! constants/Constants 같은 패턴은 지양해주셔도 좋을 것 같아요. constatns.js 로 만들거나 constants/index.js 같이요.

Q ❓기능을 나눈 방식이 괜찮은지 궁금합니다!

• 현재 앱에서는 특정 클래스가 많은 기능을 쥐고 있어도 그 길이가 엄청나지 않기 때문에, 읽고 이해하는데 문제가 없다면 좋다고 생각합니다🤗

Q ❓파일, 폴더명 관련 컨벤션 규칙도 있는지 궁금합니다!

• 아키텍처는 개인, 팀, 회사 단위로 컨벤션을 두기도 하죠. 하지만 상황, 사용하는 스택이나 각각 비즈니스 분야에 따라 천차만별로 차이가 나기 때문에 어느것이 뛰어나다! 맞다!는 말씀드리기가 어렵네요.
• React 에서는 폴더의 깊이를 4depth 이상 내려가는 것을 지양하고 있습니다. 저는 여기에 저만의 레이아웃 규칙을 많이 정립해서 회사에 적용시키고 있어요.
• 현재 구조에서는 크게 폴더 레벨로 이야기할 부분이 없는데, 앱이 커지면 꼭 리뷰어분께 요청해주시길 바랍니다.

커링...은 지나쳐도 좋습니다! 현재 접해도 어렵고 난해하여 야미가 고민하는 방향성을 흐트릴 수가 있다고 생각되었습니다. redux 미들웨어가 이 기법을 잘 사용해서 좋은 참고 자료가 될 거에요. 그래도 언급은 했으니...🥲 괜한 내용을 말해서 야미를 고통스럽게 한 건 아닌가 싶네요. 아래처럼 커링을 적용하면 됩니다!

const errorCatcher =
  (validator, errorCallback) =>
  (...args) => {
    try {
      validator(args);
    } catch (error) {
      errorCallback(error);
    }
  };

const safeCall = errorCatcher(
  (param) => {
    console.log('!!!', param);
    throw new Error('헉 이상한 에러가 발생했어요!');
  },
  (error) => console.log('!!!에러!!! : ', error),
);

safeCall('이게 실행이 될까요?');

• 이를 적용하면 좋을 것이다~ 라는 의미는 HOF로 함수가 좀 더 이쁘게 호출되는 경험을 할 수 있기 때문이었어요. 불필요하게 함수가 여러번 인자로 호출되었으니까요... 하지만

!!!! 무시하셔도 됩니다 !!!!

어렵고 복잡하게 쓰는 것이 좋은 코드라고 할 수 없어요. 부디 이런 기법을 알았다고 너무 적용하려고 하거나 하지 않았으면 좋겠습니다.

미션 고생하셨어요! 코드는 머지하겠습니다.
궁금한 내용이 있다면 코멘트나 슬랙으로 소통하도록 해요🤗

[InSeong-So]

min, max 값에 default Value를 선언해서 상수에 정의한 수를 주입하면 좀 더 확장성 있게 만들 수 있을 것 같네요!

Suggested change

	const randomNumberBetween = (min, max) => {
	return Math.floor(Math.random() * (max - min + 1) + min);
	};
	const randomNumberBetween = (min = RANDOM_NUMBER_RANGE.MIN, max = RANDOM_NUMBER_RANGE.MAX) =>
	Math.floor(Math.random() * (max - min + 1) + min);