배경지식!
01. 2단계는 웹으로 인터페이스를 만들어야해서 html, css를 연결해야 했다.
02. 기간은 길었고 많은 리뷰를 받으려면 빠르게 했어야 했다.
03. 바닐라 자바스크립트로 한 경험이 적어서 일단 index.js로 몰아넣었는데 후에 이걸 분리하기가 어려웠다.
04. 다른 사람들은 뷰를 만들거나 기능 단위로 구분하거나 했는데.. 나느..
1단계 PR 리뷰의 리뷰
일단 1단계를 페어 끝내고 받은 피드백들과 질문에 대한 답변은 아래와 같다.
1. 작성 의도
1) mvc패턴에 대한 고민
: 코치님은 본 미션에서 나눈다면 어떤식으로 나누실지 궁금합니다.
> M (모델): 데이터와 비즈니스 로직을 관리
V (뷰): 레이아웃과 화면을 처리
C(컨트롤러): 명령을 모델과 뷰 부분으로 라우팅이라는 기준에 맞게, 뷰를 통해서 로또번호를 받는 부분과, 결과를 보여주는 부분을 작성하고, 모델을 통해 로또 번호와 유저가 입력한 번호를 관리하고, 컨트롤러를 통해 입력시 벨리데이션 체크 및 로또 번호가 몇개 맞았고, 얼마의 수익을 얻는지 계산하는 로직을 짤것 같습니다.
대부분의 크루분들이 MVC 패턴에 대해서 너무 큰 고민을 가지고 있는데, 너무 어렵게 생각하지 말고, 정답은 없으니 MVC 를 작성하는 목적에 초점을 맞춰서 각각의 관심사를 분리해서 개발을 좀더 생산성있고, 효율적이고 유지보수성이 높게 하기 위해 고민하면서 개발하시면 좋을거 같아요 :)
반복적으로 mvc패턴에 대한 이야기가 나오고 뷰가 무엇인지 고민을 하게 된다.
설계를 하는 첫 시작단계라서 더욱 그런가보다. 리뷰어 분께서 말씀주신대로 나도 코드를 짰고 어느정도 맞다는 부분에서 너무 이상한 방향으로 생각하지는 않았구나 하는 마음도 들었다. 계속된 구조 고민에 골치가 아팠는데, 리뷰어 말씀대로 너무 어렵게 이건 딱 뭐고 이건 딱 뭐고 생각하지 말고 효율성과 유지보수성을 생각하면 되지 않을까 생각이 들었다.
2) TDD 작성
: validation 외에는 오류가 발생하는지 테스트를 하고 있지 않습니다. 테스트 내에 들어오는 값은 이미 완벽하다고 가정하고 진행을 하는데 이렇게 하는 것이 맞는지 궁금합니다.
> 네 맞습니다. 사이드 이펙트를 주는 로직의 영향이 없다고 생각하고 테스트 전략을 세웁니다. 예를 들어서 api를 활용하는 로직은 msw 를 이용하여 api가 무조건 성공했다고 가정하고 테스트하거나, 사이드 이펙트를 주는 함수를 다른곳에서 따로 테스트를 짜고, 해당 함수는 무조건 영향을 안준다 가정하고 테스트를 작성합니다.
3) 객체 안 변수 밖으로 꺼내지 않기
: 하나의 객체에 요구하는 것이 많아진다면 그것을 객체 내 각 함수로 구현하여 반환해야 하고, 그렇다면 객체가 너무 무거워지지 않을까 생각이 들기도 했습니다.
> 말씀하신대로 하나의 객체에 요구하는것이 많아지면 자연스럽게 로직이 복잡해지고 에러가 나기 쉬우며 유지보수하기 힘듭니다. 그래서 이렇게 고민이 될때는 미리 분리하는것도 좋지만 우리의 우려와는 다르게 더이상 복잡해지지 않을때가 있습니다. 이럴때는 관리나 구조측면에서는 수아가 현재 작성한 구조가 더 좋을지도 모릅니다. 그래서 이렇게 정답이 없고 고민될때는 크게 고민하지마시고, 현재의 상황에서 가장 좋은 방법을 찾고 추후 기획이 추가되고 그에 따른 코드가 변경되면서 실제로 우려하는 일을 만났을때 구조 분리를 고민하는것도 좋은 방법입니다
🙂
상황에 맞게 라는 말은 요리에서 적당히, 조금이란 말처럼 많이 해본 사람들은 알지만 초심자는 모르는 마법의 단어같다.
절대적인 것이 없기에 늘 상황에 맞춰 바꿀 수 있다는 것을 안다. 그래서 더 어렵게 느껴지기도 한다.
그래도 객체가 무거워질 수 있다는 걱정 자체는 할 수 있다는 점에서 접근이 나쁘진 않았다고 느껴졌다.
더이상 복잡해지지 않는 때가 어느때인지 궁금하다.
4) 재사용 가능하도록 만들기
: 인수가 많아지지 않을까 생각이 듭니다. 그런 경우 재사용이 어려운 함수로 생각하고 짜야하는 것인지, 혹은 제가 잘못짜서 그렇게 된 건지, 혹은 그렇다고 하더라도 재사용성이 높은 함수가 좋은건지 궁금합니다.
> 숫자를 하드 코딩이 아닌 인수로 나누는 기준은 크게 2가지로 생각해주시면 됩니다.
1. 코드 관리 측면에서 이 숫자의 의미가 명확하지 않을 경우 인수로 빼두시는게 좋습니다.예를 들어서 if(lotto > 3000) 이라는 숫자를 직접 적기 보다는 const lottoPrice = 3000 두고 if(lotto > lottoPrice) 이 더 좋습니다.
2. 기획단계에서 쉽게 바뀔 수 있는 수는 인수로 빼두시는게 좋습니다.예를 들어서 최소 금액 1000원이라는 수는 언제 든지 변경할 수 있습니다. 하지만 이러한 1000은 여러 곳에서 쓰일수 있기 때문에 인수로 관리 하면서 수정사항이 생겼을경우 한곳만 수정하면 되서 에러에 취약하지않습니다.
이렇게 인수를 잘 활용한다면 생산성을 물론 가독성 측면에서 큰 이득이 있기때문에 인수가 너무 많아질 걱정은 안하셔도 될거 같아요. 만약 그래도 너무 많아지는게 고민이라면 인수를 한곳에서 관리하지 않고 인수라는 폴더를 두고 관련있는것끼리 파일로 관리해도 좋은 방법일거같네요 🙂
재사용성은 어렵다... 후.. 그래도 위 두 기준을 읽으니 대충 내가 뭐가 문제인지 알 것 같다.
나는 지금까지 정해진 조건을 지키는 미션을 해왔다. 그러니 그 조건이 변경이 될 것이라고 생각을 해본 적이 없는 것이다.
이 플젝에 무엇이 엎어질지, 뭐가 수정될지 경험한 적이 없으니 고려해본적도 없는 것이었다.
이건 감이 없는 문제여서 다른 사람들 코드를 보고 어느걸 유틸로 빼낼 수 있었을지, 상수화 할 수 있었을지 고민해야 할 것 같다.
-
추가적인 코드리뷰
[ 조언 ]
- 한줄인 함수 = const AMOUNT_OF_PURCHASE = (amount) => `${amount}개를 구매했습니다.`; 처럼 만들기
- Const 하고 return 변수하는 거면 걍 return 담에 바로 적는거
- Map - forEach의 사용법 잘 구분하기
- if문에서 return값이면 중복이면 그냥 조건 합쳐서 반환하기
- Test는 중복된 코드면 집약적으로 test.each같은거 사용해서 한번에 하기
- Object.freeze();으로 상수 얼리기
- 단순 숫자 상수화 시키기
[ 공부하면 좋을 것들 ]
- Sort의 동작방법
- 인덱스/순서에 의존한 배열순환은 오류가 발생할 가능성이 높으므로 좀더 명확한 데이터에 의존하는 법을 고민해보기
[ 칭찬받은거 ]
- set으로 중복확인
2단계 진행
html과 css를 연결해서 기존의 도메인을 연결해야 했다.
이전에는 뷰와 도메인을 컨드롤러에서 연결을 해서 인덱스에서 컨트롤러를 불러 시작을 했다면,
이번에는 인터페이스와 인덱스를, 인덱스와 컨트롤러를, 컨트롤러가 로또게임을 불러 진행을 했다.
이전에는 컨트롤러가 중심을 잡았다면
이번에는 인덱스가 모든 것을 다 가지고 가는 느낌이었다.
음 일단 그러한 고민을 피알에 담았으니 피알 메세지를 옮겨오겠다.
1. 도메인과 UI
: 1단계에서 도메인과 UI를 분리하고자 했습니다. 해서 이번에 인터페이스로 UI가 바뀌며 input/output은 사용하지 않고 index와 controller만을 수정해 " html - index.js - contreller - 기타 도메인 "으로 연결되게 만들었습니다. 이렇게 되니 controller가 도메인만을 담당하게 되며 controller의 기능을 잃게 된 것 같아 아쉽습니다. 이전에 mvc를 신경썼다면 html을 연결하는 파일을 생성해 controller와 연결을 했어야 하는 생각도 듭니다.
2. step2-index.js
: 해당 파일에서 모든 UI처리를 합니다. 바닐라 자바스크립트에 익숙하지 않아 구동이 되게 작성하는 것을 우선순위로 두다 보니 한 파일에 하게 되었습니다. 이후 분리를 하고자 했지만 어떤 기준으로 해야 할지 고민이 들었습니다.'inputView/outputView를 만들어 input에는 이벤트리스너와 입력값을 받는 것, output에는 화면에 내보이는 것'으로 하는 크루들도 있었습니다. 제 코드는 초반에 이벤트리스너를 부여하고 이후 이벤트리스너에 쓰이는 함수를 차례로 적기 때문에 도메인 함수를 부르는 경우도 있어 분리가 어려웠습니다. 제 생각 역시 하나의 화면에서 연속해서 처리되는 과정을 무슨 기준으로 input과 output으로 명료하게 나눌 수 있는지 의문이 들었습니다.'화면(기능) 별로 분리하여 로또값을 받는 메인 페이지와 결과를 띄우는 모달창'으로 구분한 크루도 있습니다. input과 output으로 나눈 것보다는 해당 의견이 더 적절하다고 생각했는데, 하나의 이벤트와 그에 연결되는 함수들을 한눈에 보이는 것이 더 가독성이 좋지 않을까 생각했습니다. 더불어서 콘솔과 다르게 이벤트 시 html 태그의 속성을 변경해야 하는 상황에서 이 편이 더 처리가 용이하다고 생각했습니다. 하지만 이미 한 파일로 진행을 하여 분리를 하는 경우에 함수가 꼬이는 경우가 생겨버려 일단 처음 작성한 파일로 제출합니다.결과적으로 아직 어떤 기준으로 파일을 분리해야하는지 잘 모르겠습니다. 인터페이스와 상호작용하는 경우가 처음이라 이걸 여러 파일로 나누어도 되는 것인지, 이벤트 별로 나누는 것이 좋을지, 화면 별로 나누는 것이 좋을지, 혹은 다른 기준이 있을지 궁금합니다.
3. index.js / app.js (궁금한점): 일전에 위 두 가지 개념을 찾아 본 경험이 있습니다. 제가 해석하기로는 index.js는 라우팅과 관련한 공간으로 기능구현으로 들어가는 문같은 느낌의 파일로, app.js는 기능구현을 계층화했을때 가장 상위의 파일로 이해했습니다. 제가 라우팅을 잘 이해하지 못해서 html 태그를 가지고 오는 것도 라우팅의 일종이지 않을까 생각을 해 index.js에 작성을 했습니다. 그런데 많은 사람들이 index.js 또는 app.js를 클래스로 만들어 거의 비워내고 컨트롤러를 부르거나 앱을 부르는 용도로 사용하며 통로처럼 사용한다고 느껴졌습니다. 이러한 느낌이 저 혼자만의 느낌일 수도 있고, 혹은 일부분일 수 있지만 혹시 이에 대해 알고 계시거나 혹은 제가 잘못 알고 있다면 알려주셨으면 좋겠습니다.
4. 커밋 메세지: 이전 피드백에서 커밋메세지의 통일성을 말씀해주셨습니다. 아직 커밋메세지나 함수명, 변수명 등을 작성하는 것이 어렵습니다. 특히 이번에는 html, css를 기능구현이라고 할 수 있는지 고민을 했는데, 기능이라고 생각을 해서 feat을 했습니다. 커밋메세지를 쓰며 무엇을 위한 변경인지, 어디를 어떻게 고쳤는지를 최대한 자세하게 쓰는 것이 좋은지, 작성/수정한 함수명등을 포함하는게 좋은지, 기능구현목록에 적힌 정도로만 작성을 하면 좋을지 팁을 알려주시면 도움이 많이 될 것 같습니다! :)
5. 오류처리: 이번 미션에서는 오류를 처리해서 alert창으로 띄웠습니다. 이 과정에서 유효성 검사를 포함한 함수에서 return 값이 없으면 오류처리가 된 것으로 처리를 해서 다음 진행이 안되도록 만들었는데 모든 인풋을 이렇게 오류처리하기엔 무리가 있다고 생각이 들었습니다. 유효성 검사에 걸리면 다음 진행이 되지 않도록 하는 다른 방법이 있을지 궁금합니다.
결국 js에 대한 수정보다는 일단 그 외 것에 집중을 했고, 그 과정에서 생기는 궁금증들을 여쭤봤다.
그리고 피알을 보내고 다른 크루들이랑 이야기를 하며 내가 굉장히 쉽게 생각을 해서 빨리 끝냈구나 하는 점을 알게 되었는데,
예로 모달창에서 배경화면을 클릭했을때 창이 닫히지 않는거나, 엔터를 탭으로 이동해 엔터를 눌렀을 때 이벤트 발생이 되지 않거나...
사실 이맘때에 구조에 대한 고민이 있어서 에라 모르겠다하고 그냥 넘겼던 것 같다.
내고 나니 조금 부끄러워지는 코드였다.
그렇다고 돌아가서 다시 더 하고 낼래? 하면 그냥 내버린게 나을 것 같다.
다음주에 테코톡이다 스터디다 이러니까 마음이 조급한 것 같다.
'2022우아한형제들_우테코' 카테고리의 다른 글
[우테코] Lv. 1 로또 구현 - 2단계(웹) 2차 PR 후기_최종 (0) | 2023.02.27 |
---|---|
[우테코] 어쩌다 면담 (0) | 2023.02.24 |
[우테코] Lv. 1 로또 구현 - 1단계(페어) 후기 (0) | 2023.02.18 |
[ 우테코 ] Lv. 1 자동차 경주 구현 - 2단계(리팩토링) 후기 (0) | 2023.02.13 |
[우테코] Lv. 1 자동차 경주 구현 - 1단계(페어) 후기 (0) | 2023.02.09 |