search

S0YKIM / JS-Counter

A simple counter made with vanilla js
1 stars 0 forks source link

[Study] Feedbacks #1

Open MichelleJin12 opened 3 years ago

MichelleJin12 commented 3 years ago
  1. 이문제는 다 나는거같은데 console에 favicon.ico 에러가 난다.. 왜인지는 모르겠..
  2. 버튼 누를 때 색이 바뀌거나 클릭되는 기능이 있어서 누를 때 쫀득한 손맛이 있으면 좋겠다.
  3. html에 counter 형식을 미리 넣지 말고 js에서 넣어주면 html이 더 간결해질 것 같다 (이게 더 좋을지는 모르겠네욥 js 로딩이나 그런걸 고려한다면?)
srngch commented 3 years ago

index.html:13 기능적으로는 크게 상관은 없지만 article보다 main이나 section을 추천합니다! https://validator.w3.org/ 에 따르면 section은 제목 태그를 가져야 좋기 때문에, 저는 main을 사용했습니다. 태그 별 추천 상황은 문서 참고해주세요! https://developer.mozilla.org/ko/docs/Web/HTML/Element/article https://developer.mozilla.org/ko/docs/Web/HTML/Element/main https://developer.mozilla.org/ko/docs/Web/HTML/Element/section

style.css CSS 속성에 순서가 있다면 읽기 편합니다. 자세한건 링크로... 저도 빡빡하게 지키고 있지는 않지만 습관적으로 여백>위치>타이포>배경>기타 순으로 사용하고 있습니다. https://beautifulcss.com/archives/203 https://www.howdy-mj.me/css/order-of-css-properties/

index.js:34 클래스 인스턴스 객체의 프로퍼티에 _ 를 붙이는 것은 비공개 속성(외부에서 접근하지 않음)으로 사용하겠다는 일종의 관습이기 때문에 Counter 클래스에서 _를 때고 count라고 명명하거나, (이후 기능 추가를 대비한 것이라면) getter를 따로 만들어주는건 어떨까요?

나머지는 깔끔해서 넘 좋습니다..

hhkim0729 commented 3 years ago
  1. 먼저 짧은 시간에 html, css 공부도 하신 걸 알 수 있었어요. 바로 flex 활용까지 하시다니 👏
  2. 일반적으로 js, html, css 인덴테이션은 스페이스 2번을 많이 쓴다고 해요. 가장 많이 쓰는 code formatter인 prettier의 기본 옵션도 그랬던 것 같고..? 👉 혹시 prettier 쓰고 계시면 설정 한번 들여다보시고 아니면 다음에 써 보시는 게 좋을 것 같습니다. (index.css:12 탭 2번 들어간 걸 보면 아마 자동 포매팅이 안 되고 있는 것 같네요.)
  3. counter.js 사랑님도 말씀하신 것처럼 Counter 클래스 내부 변수를 프라이빗으로 만드셨으니 각 메서드에서 _count를 리턴하기보다는 getter를 만들어주시는 게 좋을 것 같습니다~!
  4. index.js 클래스를 알게 되었으니 App도 클래스로 선언해보시는 것을 제안합니다..호호 function App이 결국 클래스 개념으로 쓰이고 있으니까 내부 counter 변수도 this.counter가 되는 게 맞지 않나 싶네요. 아니면 30번째 라인 이후 내용(카운터 인스턴스 생성 및 이벤트 리스너 연결)도 결국에는 init하는 내용이니까 init 함수에 넣어 보는 것은 어떠신가요? => 이 과정에서 이벤트리스너를 묶어서 메서드로 빼면 좀 더 깔끔하겠쥬
  5. 전체적으로 기능도 문제없이 작동하고 코드도 깔끔하게 짜셔서 한 눈에 보기가 편했어요~! 개선점을 더 쥐어짜보자면 문벅스 강의에서도 그랬지만 파일 나누기 연습도 해보시면 좋을 거 같아요 👍 또 개인적으로 카운터 여러 개 생성하는 보너스도 많이 도움이 되었기 때문에 언젠가 다시 도전해보시길 추천드립니다!
dopamingo commented 3 years ago

  1. index.html 전반적으로 id를 너무 많이 사용했어요 class 사용하는 것을 추천합니다!

  2. index.js:5 querySelector 저도 다음엔 이렇게 해봐야겠어요 ^.^

  3. 클론해서 작동시켜봤는데 value 가 로컬에 저장되고 새로고침하면 잘 불러와 집니다 그러고 +, - 버튼을 누르면 다시 0으로 초기화되네요... 아직 이 부분은 안 하신 거겠지요?

전 아직 문벅스 강의 안 봤는데 이게 많이 도움되셨나봐요!! 봐야겠다 . . . 후후 고생 많으셨습니다 ~!