[2단계 - 자동차 경주] 버건디(전태헌) 미션 제출합니다. #294
Conversation
There was a problem hiding this comment.
원래 목적은 validateFunctions 라는 유틸 함수를 만들어서, 다음 미션에서도 재사용하기 위함이었는데 AppError와 ERROR_MESSAGE 상수 파일과의 의존성을 갖게 되었네요🥲
step1 때는 Validator라는 클래스로 만들었었고, 여기서는 유틸함수로 분리를 해본것인데 뭐가 더 괜찮을지 한번 여쭈어보고싶습니다 !
src/views/InputView.js
Outdated
| const InputView = { | ||
| async inputCarNames() { | ||
| const names = await asyncReadLineHandler(PROGRESS_MESSAGE.INPUT_CAR_NAMES); | ||
| return names.split(DELIMITER.COMMA).map((name) => name.trim()); |
There was a problem hiding this comment.
view를 담당하는 객체에서 현재 자동차 이름을 입력값을 받을때, ","를 기준으로 나누어주고 앞뒤 공백을 없애주는 파싱작업을 하고 있는데요.
여기서 유효성 검증 로직에 관여하거나 하지 않고, 단순히 파싱만 해주는것이다보니 InputView 안에서 해주어도 되지 않을까? 라는 생각이 있는데, 이에 대한 의견을 한번 여쭈어보고싶어요 !
There was a problem hiding this comment.
(이전에 댓글 달았다가 잘못 이해해서 지우고 다시 달았어요.)
View는 이미지로든 텍스트로든 출력되어 사용자에게 보여주는 것일텐데, InputView의 역할은 무엇으로 보고계실까요~? 역할에 대한 정의에 따라 판단해주시면 좋을 것 같습니다.
src/App.js
Outdated
| await asyncFunctionHandlerWithError(this.#readCarNames, this); | ||
| await asyncFunctionHandlerWithError(this.#readCountOfAttempts, this); |
src/App.js
Outdated
| } | ||
|
|
||
| #printWinner() { | ||
| const finalWinnerArr = Game.getFinalWinner(this.#gameResult); |
There was a problem hiding this comment.
항상 변수명 지을때 헷갈리는 부분이었는데, 짚어주셔서 감사합니다!
여기서 최종 우승자라는 것이 한명일수도, 여러명일수도 있는 상황에서 저같은 경우에 s 를 붙여서 복수형 표기를 해주는것이 맞을까? 라는 생각을 했었는데요.
그래서 Arr이라고 혼자서 변수명을 지었었습니다!
일반적으로 단수일수도, 복수일수도 있는 상황에서는 s를 붙여주는것이 더 맞는 상황이라고 이해해보면 될까요 ??
| @@ -1,7 +1,8 @@ | |||
| import deepFreeze from '../../utils/deepFeeze.js'; | |||
|
|
|||
| /* eslint-disable no-useless-escape */ | |||
| const REGEX_CONFIG = deepFreeze({ | |||
There was a problem hiding this comment.
영어에 대한 지식이 많지가 않아서, 상수 카테고리 내에서 설정 값이라고 할수 있는 부분에 대한 범주에 들어간다면 설정의 의미를 가진 CONFIG를 붙여주었습니다!
예를 들어서 RANDOM_CAR_NAME_CONFIG.DEFAULT는 랜덤한 자동차에 대한 기본적인 이름 설정값 을 생각해서 만들어본것인데요.
하루 말씀 듣고보니, config 로 들어가야하는 값들은 이런 상수가 아닌 변동성이 있는 값(?)들을 다루어주어야하나 ? 라는 생각이 지금 들었습니다!
그래서 지금 대체할만한 단어로 생각해본것은 OPTIONS 나 CONSTANTS 인데요. 혹시나 더 나은 대체할만한 단어가 있다면 말씀주시면 반영해보겠습니다 �🫡
| @@ -0,0 +1,11 @@ | |||
| import deepFreeze from '../../utils/deepFeeze.js'; | |||
There was a problem hiding this comment.
constants
┣ configs
┃ ┣ gameCondition.js
┃ ┣ randomCarNameConfig.js
┃ ┗ regexConfig.js
┣ delimiters
┃ ┗ delimter.js
┗ messages
┃ ┣ errorMessage.js
┃ ┣ progressMessage.js
┃ ┗ resultMessage.js
constants 폴더내의 계층구조가 현재 이런식으로 잡혀있는데요!
delimiter 말고는 현재 다른 것들은 여러개의 파일로 나누어져있는데, 저 delimiter 만 따로 contants 폴더 바로 안에 존재한다면 폴더 구성이 어색하지 않을까 ? 라는 생각을 했었습니다.
하루 말씀대로 delimiter는 delimiter 말고 따로 들어갈 파일이 존재하지 않는것 같긴하네요..!
이런 경우에는 단일파일로 그냥 constants안에 넣어주는게 더 나은 방법일까요 ?
| @@ -0,0 +1,9 @@ | |||
| import deepFreeze from '../../utils/deepFeeze.js'; | |||
|
|
|||
| const DELIMITER = deepFreeze({ | |||
| import AppError from '../../errors/AppError.js'; | ||
| import ERROR_MESSAGE from '../../constants/messages/errorMessage.js'; | ||
|
|
||
| export const validateMinLength = (inputValue, conditionValue) => { |
There was a problem hiding this comment.
🥲🥲 하루 말씀이 맞다고 생각합니다. 한번 다시 수정해보도록 하겠습니다!
| } | ||
| }; | ||
|
|
||
| export const validateRequire = (inputValue) => { |
There was a problem hiding this comment.
빈 문자열을 체크하기 위해서 만들어놓은것이었는데, 결과적으로 쓰인곳이 없었네요 🥲
짚어주셔서 감사합니다! 반영하겠습니다. (다음부턴 함수명도 고심해보겠습니다!)
src/utils/asyncReadLineHandler.js
Outdated
|
|
||
| function asyncReadLineHandler(query) { |
src/validator/Validator.js
Outdated
| @@ -0,0 +1,37 @@ | |||
| import { | |||
There was a problem hiding this comment.
errors 안에 들어가는건 커스텀 에러 객체이고, validator는 유효성 검증을 담당하는 객체인데,
나머지 domain, views, utils 안에 들어갈수 없다고 판단해서 따로 폴더를 만들었습니다!
혹시 수정해야할 부분이나, 제가 잘못 짚고 있는 것이라면 말씀 부탁드립니다! :)
| } from '../utils/validate/validateFunctions.js'; | ||
| import GAME_CONDITION from '../constants/configs/gameCondition.js'; | ||
|
|
||
| const Validator = { |
There was a problem hiding this comment.
클래스를 사용함으로써, private 필드를 만들어서 은닉화를 할수 있고 super()나 extend 를 이용하여 상속을 할수 있다.(?) 정도로 알고 있는데요..!
하루가 질문 주신 기회에 이번에 블로그에 따로 한번 더 정리해보도록 하겠습니다 !
키워드 던져주셔서 감사합니다 :)
안녕하세요 하루!
1단계에서 리뷰 남겨주신 부분들 2단계에서 한번 반영해보았습니다.
- 📌 신경 쓴 부분
1. try- catch의 공통된 구조를 가진 입력갑 함수를
asyncFunctionHandlerWithError라는 유틸함수로 분리step1 에서는 입력값을 받는 함수마다 App.js 에서 try-catch 함수를 이용하여 입력값 기능을 구현해주었는데요!
이번에는 유틸함수로 하나로 묶어주어서, 코드의 가독성을 높이고자 해보았습니다 :)
2. Validator를 클래스가 아닌 객체로 수정
유효성 검증을 step1에서는 Validator 클래스에서 static 메서드를 사용하여 구현해주었었는데, 전부 static 메서드이다보니 클래스 선언의 장점을 누리지 못하는것 같다고 판단했습니다.
그래서 이번에는 일반 객체로 분리해서 검증해주었습니다!
utils 내부에서 validateFuntions 라는 검증 함수 모음집을 하나 만들어주어서 Validator 객체 안에서 검증이 필요한 부분들을 부분적으로 impor해와서 사용했습니다 :)
3. OutputView의 역할 경감
step1에서는 OutputView 내에서 경주 결과를 담은 Map 객체를 받아서, 객체 함수 내부에서 파싱작업을 해서 최종 승자를 출력 해주었는데요.
이번에는
getFinalWinner라는 도메인내의 메서드를 만들어주어서, 결과값을 담은 배열만 리턴해준후, OutputView에서 간단히 출력해보았습니다 :)4. 임의의 자동차 이름 부여
step1에서 말씀드렸지만 구현 하지 못했던, 사용자가 ""같은 빈 문자열을 입력했을때,
user1같은 임의의 값을 부여해주는 기능을 구현해보았습니다.makeRandomCarName이라는 서비스 함수를 하나 만들어서, 유효성 검증 전에 빈 문자열이라면 user1, user2 의 값을 구현해보았어요 !하지만 현재 문제점은 user10이 됐을때는 6글자가 되어버려서 기능 요구사항인
자동차 이름은 5글자를 넘지 않는다의 조건을 벗어나게 되네요..!이에 대한 해결책으로는 아예 자동차 참가자 수 자체를 제한하는 기능 사항을 하나 추가해야할것 같은데 이에 대해서는 대응을 아직 하지 못한 상황입니다..! 🥲
5. eslint 및 prettier 적용
말씀해주셨던 eslint 와 prettier 를 적용시켜보았습니다! (하루 말씀대로, npm run lint 하자마자 빨간줄이 와장창 나오더라구요.. step1때 답답하셨을텐데 양해해주시고 코드 봐주셔서 감사합니다.)
현재 단순 입력값을 받아주는
asyncReadLineHandler는 readline 모듈을 불러오는 이유로 인하여 함수가 10줄이 넘어간 상태인데요 !이는 아무리 줄이려해도 10줄이하로 작성할수가 없어서 일단 냅둔 상태입니다!
6. 매직넘버 상수처리
최대한 매직넘버들을
constants폴더 안에서 상수로 만들어주어서 상수처리를 해보았습니다!이외의 궁금증 같은건 git comment 로 남겨보도록 하겠습니다!
감사합니다!