-
Notifications
You must be signed in to change notification settings - Fork 181
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기 소인성] 자동차 경주 미션 STEP1 ~ 3 #62
Conversation
- ESLint, Prettier, @babel/eslint-parser - Cypress
- 테스트 케이스 정의
- 인터페이스를 Proxy로 구현
- 도메인 모델 정의에 대한 컨벤션 작성 - 함수형 파이프라인 추가
- 의존성 주입 방향 설정 - 이벤트 바인딩 엘리먼트 함수 rest로 받도록 수정
- carNames validation 추가 - 유효성 검사 로직 분리, 에러 메세지 추가
- diff 알고리즘 추가 - publish/subsribe 패턴 적용
- view를 담당하는 함수가 컨트롤러 인자를 받아 인스턴스(상태)에 따른 엘리먼트 변경이 되도록 수정
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.
안녕하세요, 리뷰어 정재남입니다.
구조에 많은 신경을 쓰신 것 같아요. 그런데 그러다보니 코드 복잡도가 너무 올라간 것 아닌가 싶습니다. 서비스 규모에 비해 코드를 분석하고 이해하는 데에 드는 노력이 많이 요구되어 리뷰하기가 참 어려웠네요.
여러가지 욕심을 내신걸로 보여요. virtual dom - real dom 비교, pub/sub, functional, prototype, proxy... 이런 것들이 이번 미션의 요구사항을 구현하는 데에 있어 필요했던 것인지, 혹은 그만한 가치가 있는지를 생각해보시고, 가능한 오버엔지니어링을 지양해 주시면 좋겠습니다.
제가 정신이 혼미하여 코드 전반에 대한 리뷰는 드리기가 어렵고, 몇가지 코멘트는 남겼어요.
src/js/@helper/valid.js
Outdated
@@ -0,0 +1,25 @@ | |||
export const isNil = value => value == null; |
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.
lodash의 영향을 받으신 걸까요? 그런데 lodash의 isNil 함수는 null뿐이 아닌 undefined도 체크를 합니다.
그렇지 않고 오직 null만 체크하는 목적이라면 isNull
이 맞지 않나 싶어요.
나아가 ==
보다는 ===
로 비교하는게 형변환을 하지 않아 성능상 더 좋다고 합니다.
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를 수행하면서 isNul
l과 isNil
을 어떻게 해당 앱에 녹일까를 고민하다가 포괄적인 isNil선택했었습니다.
리뷰어님 말씀대로 null만을 비교하기 위해선 isNull이 옳았지만 인자 존재 여부를 조건문의 !
연산으로 체크하는 게 아니라 명시적으로 표현하고 싶었기 때문입니다. 사용 결과 최대한 억제하는 게 옳아 보입니다!
src/js/@helper/dom/diff.js
Outdated
export const diff = ($element, realNode, virtualNode) => { | ||
if (isNil(realNode) && isNil(virtualNode)) return; | ||
if (isOnlyExistRight(realNode, virtualNode)) return $element.append(virtualNode); | ||
if (isOnlyExistLeft(realNode, virtualNode)) return realNode.remove(); | ||
if (isChangedNode(realNode, virtualNode)) return realNode.replaceWith(virtualNode); | ||
|
||
const [realChildren, realLength] = getChildren(realNode); | ||
const [virtualChildren, virtualLength] = getChildren(virtualNode); | ||
|
||
for (let i = 0; i < Math.max(realLength, virtualLength); i++) { | ||
diff(realNode, realChildren[i], virtualChildren[i]); | ||
} | ||
}; |
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.
다른 경우에는 괜찮지만, isOnlyExistRight의 경우에는 diff가 제대로 수행될지 의심이 듭니다.
$element | real | virtual | result |
---|---|---|---|
[li.a, li.b, li.d] | [li.a, li.b, li.d] | [li.a, li.b, li.c , li.d] |
[li.a, li.b, li.d, li.c ] |
[li.b, li.c, li.d] | [li.b, li.c, li.d] | [li.a , li.b, li.c, li.d] |
[li.b, li.c, li.d, li.a ] |
이렇게 되는거 아닐런지..
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.
테스트해보니 리뷰어님 말씀대로 노드 변화에 따른 diff 수행이 제대로 되고 있지 않네요. DOM 요소가 innerHTML 처럼 동작하고 있어서 의미 없는 비교가 되었습니다. 추후 많은 수정을 거쳐야 할 것 같아요.
diff를 넣은 것은 리뷰어님들의 리뷰를 받아보기 위함이었으니, 다음 피드백 때 해당 미션에 맞는 엔지니어링을 진행해볼게요.
src/js/@helper/dom/diff.js
Outdated
const getChildren = target => { | ||
if (!target) return [[], 0]; | ||
const targetChildrens = Array.from(target.children); | ||
return [targetChildrens, targetChildrens.length]; | ||
}; |
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.
children과 length를 별도로 제공하는 이유가 궁금합니다.
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.
코드를 조금 더 깔끔하게 만들어볼 수 있지 않을까 하는 의도에서 탄생했던 형태입니다.
src/js/@helper/dom/diff.js
Outdated
|
||
if ($realNodeAttributes.length !== $virtualNodeAttributes.length) return true; | ||
|
||
for (let i = 0; i < $realNodeAttributes.length; i++) { |
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.
12번줄에서는 max로 기준을 잡으셨는데 여기서는 real만을 기준으로 잡은 이유는 뭔가요?
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.
max Length를 기준으로 잡았어야 했는데 놓친 부분이 있었네요😭
src/js/@helper/dom/diff.js
Outdated
for (let i = 0; i < $realNodeAttributes.length; i++) { | ||
const $r = $realNodeAttributes[i].name; | ||
const $v = $virtualNodeAttributes[i].name; | ||
if (isEqualsAttribute(realNode, virtualNode, $r, $v)) return true; |
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.
이러면 하나라도 같은 attribute가 발견되면 early return되어서 나머지 attribute들이 같은지 다른지 여부를 체크하지 않게 되는거 아닌가요?
라고 적고 45번줄로 가보니, 변수명은 isEqual이라서 '같은 경우에 true, 다르면 false'일 걸로 예상했는데, 내용은 정반대네요..
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.
혼란을 줄 수 있는 네이밍이네요. 수정했습니다☺
src/js/@helper/valid.js
Outdated
export const isFirstChar = (target, char) => target.charAt(0) === char; | ||
|
||
export const isLastChar = (target, char) => target.at(-1) === char; |
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.
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.
DOM API 활용을 적극적으로 시도해보겠습니다🙏
src/js/Application.js
Outdated
|
||
constructor() { | ||
this.#racingGame = new RacingGame(); | ||
this.#publisher.subscribe({ render: this.render.bind(this) }); |
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.
render를 arrow function으로 바꿔주시면 bind(this)를 뺄 수 있고 동시에 성능상 이점도 얻을 수 있습니다.
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,0 +1,46 @@ | |||
import RacingGameService from '../services/racingGame.service.js'; | |||
|
|||
export default class RacingGameController { |
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.
이 클래스를 없애고 racingGameService만으로 운영하더라도 충분해 보이는데 어떻게 생각하시나요?
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.
과한 래핑 중 하나라고 생각되며, 제거하는 것에 공감합니다.
src/js/domain/models/car.model.js
Outdated
moveCount: 0, | ||
}; | ||
|
||
export default () => ({ __proto__: _interface_(CarModelProps) }); |
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.
https://developer.mozilla.org/ko/docs/Web/JavaScript/Reference/Global_Objects/Object/proto
다른 방법 (setPrototypeOf 또는 Object.create)를 추천합니다.
그런데 정말 프로토타입이 필요한 상황인가요? 전체 서비스를 통틀어 인스턴스는 하나만 사용되는 것 같은데..
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.
proxy를 사용해서 타입스크립트의 타입 시스템을 흉내 내어 보았습니다. 도메인 모델을 interface로 작성해보려고 했던 시도였어요.
던더 프로토는 2주 전 세션 때 포코가 언급해주셔서 사용을 해봤습니다. setPrototypeOf나 Object.create를 시도해봐야겠네요.
const winnedCount = Math.max(...this.#racingGame.cars.map(car => car.getMoveCount)); | ||
const winnedCars = this.#racingGame.cars.filter(car => car.getMoveCount === winnedCount); |
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.
순회 한 번으로 찾아낼 수는 없을까요?
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.
고민해보겠습니다!
우선 정신이 혼미하셨다니 죄송스럽네요🥺 오버 엔지니어링이 맞았으나 좋은 리뷰어들께 리뷰를 받을 수 있는 유일한 시간이었으므로 스스로에겐 유의미한 시도였다고 생각합니다. 이번에 조금 무리하더라도 시도해보지 않았던 여러 기술들이나 구조들을 적용해보았고, 회사에서 사용하든 DDD나 헥사고날 아키텍처를 이해하기 위해 작은 규모의 앱임에도 최대한 고민하여 분리해보았습니다.
다른 분들의 커밋된 파일 수의 2배, 많게는 3배에 육박하더군요... 얼마나 힘드셨을지 짐작이 갑니다. 그럼에도 불구하고 좋은 리뷰 남겨주셔서 감사합니다. 다음 리뷰 때는 현재 구조를 전혀 기용하지 않고 앱에 맞는 규모로 코드를 작성할게요. |
- 커스텀 엘리먼트(웹 컴포넌트) 활용 - HTML DOM 요소를 적극 활용하기 - view와 events 사이에서 validation을 자동으로 체크하도록 설계하기
- 최대한 HTML DOM 요소를 활용하기 - validation을 브릿지처럼 연결하여 사용하기
- fieldset을 form으로 교체하여 UX 향상 - attributeChangedCallback를 활용하여 상태 관리... 수정해야 함 - setInterval을 사용했으나 requestAnimationFrame을 활용하도록 수정할 것
- disabled 추가
- 적용되지 않았던 css를 수정
- 파이프 라인을 구성하는 팩토리로 서비스 로직을 명명 - 불필요한 헬퍼 함수 삭제
- 공통된 로직에 대해 리팩토링이 추가로 필요 - 렌더링이 가장 문제 - requestAnimationFrame을 적용해야 함
- setInterval 대체 - async/await로 제어하며 내부 함수를 인자로 받는다.
- GameSection에 존재하던 복잡도를 제거한다. - factory 폴더를 기능에 따라 분리한다. - pipeline을 통해 프로미스로 이벤트 제어가 비동기되는 현상을 store로 해결한다.
- prettier-ignore 적용
- 100 > 1000
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.
인성님 안녕하세요! 전반적인 코드 가독성이 확 올라서 즐겁게 읽었습니다.
다만 service, factory, helper, template의 각 명칭들이 뭔가 딱 와닿지 않는 느낌이랄까, 애매하다는 느낌이 드는데, 콕집어 뭐라고 말해야 좋을지는 모르겠어요 ㅎㅎ 서비스가 내가 아는 서비스가 맞나? 무엇을 팩토리라고 하는거지? 템플릿이랬는데 템플릿이 아닌것 같은...?
위 내용 및 각 파일에 남긴 코멘트들은 개인적인 소감이나 의견일 뿐이라 크게 신경쓰지 않으셔도 될 것 같아요. 다만 테스트해보니 동작이 안되는 버그가 있는데 이건 해결해주시면 좋겠습니다.
일단은 승인하고, 버그만 해결해주시면 바로 머지하도록 하겠습니다. 고생하셨어요 :)
src/js/Template.js
Outdated
constructor() { | ||
super(); | ||
} |
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.
요건 없어도 돼요 :)
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.
삭제할게요! 최상위 웹 컴포넌트에서 무엇을 정의하여 효율적으로 사용할 수 있을까를 고민하다가 지우질 않았네요🙇♂️
src/js/App.js
Outdated
</fragment>`; | ||
|
||
export default class App extends Template { | ||
#handler = []; |
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.
21번줄, 33번줄, 그리고 Template의 'bindEvent' 내용을 살펴보면
- 13번줄의 초기값이 배열이면 안될 것 같아요. 나중에 덮어씌워지긴 하지만 제삼자 입장에선 혼란스러울 수 있으니까요.
- bindEvent함수 호출의 반환값은
removeHandler
가 의미상 더 맞지 않을까 싶어요.
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.
말씀대로 명명하는 게 최선으로 보입니다! 초기값을 배열로 할당한 건 최상위 웹 컴포넌트의 내부에 핸들러를 선언했을 때 오버라이딩을 하지 않으면 에러가 나길래 했었는데... 현재는 각 자식 컴포넌트에 저장해두고 있죠. 이 부분은 제가 놓쳤습니다! 감사합니다🤗
const attrs = [ | ||
{ attr: 'car-names', value: detail.carNames }, | ||
{ attr: 'try-count', value: detail.tryCount }, | ||
]; |
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.
attr, value로만 구성된 것이라면 Map이나 이중배열로 충분하지 않을까 하는 의견입니다.
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차원 배열로 작업해 볼게요! map은 결국 코드가 길어지거나 2차원 배열과 비슷한 형태로 전개될 것 같거든요🙏
src/js/factory/index.js
Outdated
const executor = { | ||
[CONTROLL_KEY.CAR_NAMES]: pipe( | ||
trim, | ||
trimComma, | ||
split, | ||
removeSpace, | ||
checkValidations, | ||
), | ||
[CONTROLL_KEY.CAR_NAMES_AFTER]: pipe( | ||
() => $show('#game-try-count-form'), | ||
() => $disabled('#car-names'), | ||
() => $disabled('#car-names-confirm'), | ||
() => setTimeout(() => $focus('#game-try-count'), 100), | ||
), | ||
[CONTROLL_KEY.TRY_COUNT_AFTER]: pipe( | ||
() => $disabled('#game-try-count'), | ||
() => $disabled('#game-try-count-confirm'), | ||
), | ||
[CONTROLL_KEY.GAME_BEFORE]: pipe( | ||
split, | ||
carNames => carNames.map(carName => ({ name: carName, moveCount: 0 })), | ||
), | ||
[CONTROLL_KEY.GAME]: pipe( | ||
racingWrapper, | ||
window.requestAnimationFrame, | ||
), | ||
}; |
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.
executor들을 한 데 모아놓고 key를 상수로 관리할 필요성이 있는지 의문이 들어요.
오히려 실제 호출하는 곳에서 직접 pipeline을 정의하는 편이 더 좋지 않을까 하는 의견입니다.
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.
이 부분은 아래 길게 코멘트에 대체했습니다.
최초 작성 시 재남님 의견대로 했으면 더 깔끔했을 수 있겠네요😥
각 컴포넌트가 factory
에만 의존해서 서비스 로직을 알 필요 없게 만들고자 했습니다. factory에는 try-catch로 에러도 처리하고, 서비스 로직은 pipe를 통해만 호출하도록...
하지만 그 결과 의도가 정확히 전달되지 않고 개발자들 사이의 표준을 헤집어 놓은 느낌이네요...😇
src/js/components/GameSection.js
Outdated
if (!newValue) return this.firstElementChild.classList.add('hidden'); | ||
|
||
this.#cars = pipeline(CONTROLL_KEY.GAME_BEFORE, this.getAttribute('car-names')); | ||
this.#tryCount = this.getAttribute('try-count'); |
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.
얘는 여기서 가져와야 하는건가요? 멤버에서 제외시키고 35번줄에서 직접 접근하면 문제가 있을까요?
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.
핫 맞네요...! 의미 없이 할당한 뒤에 호출하고 있었던 코드니 말씀대로 수정하겠습니다🙇♂️
src/js/components/ResultSection.js
Outdated
const winners = this.getAttribute('winners'); | ||
document.getElementById('winners').textContent = winners; | ||
// prettier-ignore | ||
setTimeout(() => alert(`이번 레이싱 게임의 승자는\n\n${winners} 입니다!\n\n✨축하해요✨`), RESULT_ALERT_DELAY); |
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.
delay 경과 전에 '다시시작하기'를 클릭하는 경우에는 얼럿이 뜨지 않도록 하는게 어떨까 해요.
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.
좀 더 사용자 직관적이게 변경하겠습니다💪
gameReset = event => { | ||
if (!event.target.matches('#game-reset')) return; | ||
this.dispatch('reset'); | ||
}; |
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.
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.
로직을 하나 빠뜨렸는데, store를 통해 관리하던 tryCount의 문제입니다. 자동차 이름들은 입력 시 재할당 되어 문제가 없지만... maxTryCount는 게임이 진행된 후에 체크하거든요.
시도 횟수가 최초 3회 > 다시 시작 후 2회 라면 maxTryCount와 같지 않으므로 winner 측정이 되지 않았고, 웹 컴포넌트의 attributeChangedCallback
로 컴포넌트를 제어하고 있었는데 갱신되는 속성값이 없어 렌더링이 이루어지지 않았어요.
store의 상태를 초기화하도록 후다닥 수정했습니다!🤗
src/js/factory/service.js
Outdated
const maxMoveCount = store.getState('maxMoveCount'); | ||
// prettier-ignore | ||
const winners = cars.reduce((result, { name, moveCount }) => { | ||
if (maxMoveCount === moveCount) return [...result, name]; |
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.
여기서 새 배열을 만들어야 하나요?
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.
배열을 만들고, 다시 join
으로 문자열로 치환하고... 불필요한 행위네요. car-names가 많아지니 리소스도 낭비됩니다.
삼항식으로 짧게 만들고 문자열로 변경했습니다!
@@ -0,0 +1,58 @@ | |||
import { DICE_RANGE, ERROR_MESSAGE } from '../constants.js'; | |||
|
|||
// prettier-ignore |
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.
이 주석이 종종 보이는데, 어떤 이유에서 무시하게끔 하신건지 궁금해요
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.
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.
요건 아마도 prettier 설정에 'printWidth'를 현재 설정보다 작게 잡으시면 해결될 것으로 보여요. 물론 완전한 해결책은 아니긴 하지만 ;)
- Template 명칭을 ComponentHandler로 변경 - 멤버 변수 handler를 removeHandler로 명시적이도록 변경 - $setAttributes 함수의 인자를 2차원 배열로 수정
- store의 상태를 초기화 시키는 메서드를 추가
- 앞선 컴포넌트 변경 사항 적용
- 게임 결과를 출력하는 형태를 일관성있게 변경하고 사용자 친화적으로 수정 - delayLoop의 기본 값을 수정 - alert 후 다시 시작 버튼을 렌더링하도록 변경
안녕하세요, 재남님! 리뷰 감사합니다. 관련 부분을 수정하여 다시 PR 드립니다!
저도 이 부분은 많은 고민을 하고 있습니다! 최초
어제 세션 이후 다른 참여자분들과 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.
깊이 고민하고 많은걸 개선하느라 너무 고생하셨습니다!! 승인 및 머지하곘습니다.
아래 내용은 이렇게 하는게 좋겠다
보다는 저라면 이렇게 해볼것 같다
정도이고, 제가 인성님 코드를 똑같이 짤 자신은 없으니..그냥 저사람은 저렇게 생각하는구나
정도로만 여겨주시길 바라며...
- factory/index => executor들을 실제 호출하는 곳들로 옮기고 나면 pipeline만 남게 되니 pipeline을 utils로 옮깁니다.
- service는... store를 끼얹은 renderer 뿐이므로, view(component)에 위치시키는 편이 좋을 것 같아요. store와 component의 의존성을 끊었다곤 하지만 그 대신 view에 직접 관여하는 service가 store와 결합했으니, 결과적으론 마찬가지 아닌가 싶거든요.
프론트엔드 개발자로서 뭘 더 심화적으로 공부하면 좋을지
이미 차고 넘치게 잘 하고 계신 것 같습니다. 생각의 깊이나 지식의 양은 저보다 월등히 뛰어나신 것 같다고 생각해요. 아마도 지금 느끼시는 갈증은 아마도 절대적 경험량의 부족에서 기인한 것이라 생각합니다. 이론을 많이 알고 있더라도 어떤 상황에서 어떤 이론을 적용할지 혹은 하지 않을지를 판단하는 데에는 경험을 통해서만 체득되는 암묵지 영역이 존재하니까요. 이러한 암묵지는 단기에 채울 수 없기에 그저 많이 고민하고 다양하게 시도해보며 시행착오를 겪는게 최고라 생각합니다. 스스로 부족하다 생각하지 마시고 그저 코딩을 즐기시길 바라요 :)
안녕하세요, 클린코드 2기 소인성 STEP1 ~ 3 제출합니다.
피드백 적용
내용
pipe
함수를 활용했습니다.구현
실행화면 바로가기
웹 VSCode로 보기