-
Notifications
You must be signed in to change notification settings - Fork 172
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[2단계 - 자동차 경주] 초코(강다빈) 미션 제출합니다. #316
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이번 요청에서는 변경사항이 많지 않네요!
eslint와 prettier 설정이 정상적으로 된 이후에 추가적으로 다시 요청 주시는 것으로 이해하고 남겨주신 질문에 답변 위주로 우선 피드백 하겠습니다.
- 디렉토리 이름만 변경해서 해당 구조로 잘 적용이 되었는지
글쎄요 초코는 어떻게 생각하시나요? 지금 구현해주신 구조가 비즈니스 로직과 보여지는 로직이 잘 분리 되어있다고 생각하시나요? 단순히 파일이 나누어져있다고 해서 잘 분리된 구조라고 하기는 어려울 것 같아요. 이전 단계의 리뷰에서 언급했듯 여러가지 방법을 통해서 그 여부를 파악할 수 있을 것 같은데, 이 과정을 선행해보고 그 결과 어떻게 판단했는지가 궁금하네요 😁
- 입력이나 예외사항은 얼마나 디테일 해야하는지
사실 감으로 익혀야 하는지 질문은 제대로 이해한 것인지 잘 모르겠어요!
제가 이해한 바로는 스펙을 얼마나 꼼꼼하게 정의해야 하는지에 대한 질문일 것 같은데,
상황, 역할, 중요도에 따라 항상 다르겠지만 실제로 앞으로 개발을 할 때에는 많은 사람들과 함께 일하면서 개발을 해야한다는 사실을 고려한다면, 정의되지 않은 스펙으로 인해서 서로 오해를 하거나 불필요한 비용이 발생하는 것은 지양해야겠죠?
내가 함께 일하기 좋은 개발자가 되려면 어떤 방식이 좋을까를 고민해보면 답이 될 것 같아요!
- prettierrc.json
이건 아마 기존에 함께 작업하던 페어의 로컬 환경에서 gitignore가 되어있는 것은 아니려나 싶네요! 이 설정이 있어야 아마 예상한대로 prettier 설정이 적용될겁니다.
연극이 아마 레벨 1에서 다른 크루들과 하는 연극을 의미하시는 거겠죠?
즐거운 시간이 되시길 바라요.
src/constants/GameRule.js
Outdated
export const MAX_ROUND_COUNT = 5; | ||
|
||
export const RANDOM_BOUNDARY = 4; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 상수의 네이밍은 왜 RANDOM_BOUNDARY가 되었을까요?
이 네이밍만으로는 그 역할을 제대로 나타내고 있는지 잘 모르겠네요!
이 기준수치가 나타내는 것이 '랜덤 경계'인가요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"0부터 9까지의 랜덤 값 중 4 이상이 나올 때 전진한다 " 라는 조건에 사용되는 전진을 위한 임계값 을 의미하고 싶었는데 의미 전달이 부족했을 수 있겠네요! 번역기의 도움을 받아보았는데 moveForwardThreshold
이렇게 수정하면 더 나을 것 같아요. 앞으로도 변수명은 바로 파악 가능하도록 깊게 생각해봐야겠습니다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
안녕하세요 초코! 자동차 경주 미션은 이만 머지하려고 합니다!
2단계는 변경사항이 많지 않아서 조금 아쉬웠네요 😢
앞으로도 다음 미션들에서 더욱 좋은 모습들 보여주세요!
수고 많으셨습니다!
2단계 요구사항
domain/ 하위의 모듈은 view/ 하위의 모듈을 의존하지 않아야 한다.
리뷰어님께
이미 1단계 리뷰를 통해 다음과 같은 리팩터링을 진행했습니다.
2단계에서 진행한 리팩터링은 다음의 변경사항을 확인해주시면 됩니다.
+) eslint와 prettier 설정이 먹히지 않는 문제가 있어 컨벤션 관련 수정이 이루어지지 않고 있습니다. 오늘은 연극 공연이 있는 날이라 일단 pr을 보내놓고 밤에 수정하는 걸로 해도 괜찮을까요?? 양해의 말씀 드려 죄송합니다..
변경사항
질문
model/
을domain/
으로views/
를view/
로 네이밍 수정만 진행했습니다. 그런데 이게 요구사항에 맞게 코드 구조가 이루어진 건진 여쭤보고 싶습니다.