[3단계 - 나만의 유튜브 강의실] 하루(김하루) 미션 제출합니다 #64
Conversation
| } from './elementValidator.js'; | ||
| import { MESSAGE } from '../constants.js'; | ||
|
|
||
| export default class StorageController { |
There was a problem hiding this comment.
피드백 반영 확인 부탁드립니다! 🙋🏻♀️ - 식별자의 네이밍은 '역할'로
기존에 ClassroomController라는 이름을 갖고있던 컨트롤러입니다. 피드백 주신 것과 같이 기획단에 변경사항이 생기면 수정될 수 있는 Classroom 대신에, 클래스가 수행하는 역할에 맞는 이름을 고민해보았습니다.
해당 컨트롤러는 AppController라고 이름짓기에는 조금 더 구체적이 역할이 있어 다른 이름이 필요하다고 생각했습니다. 해당 컨트롤러의 역할은 SearchController에서 사용자가 저장한 영상을 보관하고 관리하는 것입니다. 따라서 이를 나타낼 수 있는 StorageController라고 이름 지어보았습니다. 괜찮은 네이밍이 되었는지 궁금합니다...! 🤔🤔
There was a problem hiding this comment.
의도를 표현하기에 Classroom 보다 훨씬 나아진것 같습니다. :)
| switchWatchFlag() { | ||
| const targetId = this.targetVideo.id; |
There was a problem hiding this comment.
피드백 반영 확인 부탁드립니다! 🙋🏻♀️ - 초기화 없이 프로퍼티 접근은 위험
기존에 이 부분에서 targetVideo를 초기화하지 않고 사용하는 부분에 대해 피드백받았었습니다!
가장 처음에는 constructor에서 초기화하는 방법을 생각했습니다. 그런데 StorageModel 의 생성시점에는 targetVideo가 없기 때문에 null, undefined, 빈문자열과 같은 값으로 초기화하더라도 targetVideo.id로 접근하면 에러가 나는 것은 마찬가지이니, 클래스의 생성시점에 초기화를 하지 않고 필요한 곳에서 할당을 하되, 아래와 같이 에러처리가 될 수 있도록 변경해보았습니다. 에러 핸들링 방식이 괜찮은지 검토해주시면 감사하겠습니다!
try {
this.model.switchWatchFlag(); // *** 해당 함수
this.view.renderWatchFlagSwitchedVideo($video, isWatching);
this.view.renderNotification(message);
this.showImageNoVideo();
} catch (error) {
this.view.renderNotification(MESSAGE.REQUEST_HAS_FAILED);
}제 좁은 견해로는 null, undefined, 빈문자열과 같은 값으로의 초기화는 큰 차이가 없을 것 같다고 생각이 드는데 제 생각이 맞는지, 아니면 이 상황에서 역시나 초기화 과정이 필요한지도 궁금합니다...!
[리뷰어님의 관련 코멘트 바로가기 💬]#46 (comment))
There was a problem hiding this comment.
NPE를 if 로 잡는건 선언부에서 처리를 하는것이고 try catch 로 하는건 선언부에서 채 완성되지 않은(문제가 있는) 코드를 사용하는측에 떠넘기는 것과 마찬가지이기 때문에
NPE가 날 수 있는 부분을 try catch 로 잡기보다는 NPE가 날 수 있는 부분임이 명확하므로 if 문으로 객체 검사를 하는게 옳은 방법이긴 합니다.
| const $snackbar = $('.snackbar'); | ||
|
|
||
| export const showSnackbar = ({ message, showtime }) => { | ||
| $snackbar.innerText = message; | ||
| $snackbar.classList.add('show'); | ||
| setTimeout(() => { | ||
| $snackbar.classList.remove('show'); | ||
| }, showtime); | ||
| }; |
There was a problem hiding this comment.
피드백 반영 확인 부탁드립니다! 🙋🏻♀️ - 스낵바 표시 함수 매개변수 수정
export const showSnackbar = ({ messenger, message, showtime }) => {
messenger.innerText = message;
messenger.classList.add('show');
setTimeout(() => {
messenger.classList.remove('show');
}, showtime);
};기존 코드에서 위와 같이 $snackbar 를 첫번째 파라미터로 messager를 받고 있는 부분에 대해 피드백 받았었습니다. messenger는 $snackbar가 아닌 다른 걸 받을 수도 없고, 받을 일도 없으며, 더욱이 messenger에 적용할 CSS 클래스 'show'를 만들어놓지 않았다면 동작하지 않기 때문에 매개변수로 넘겨주는 것이 적절하지 않다는 말씀으로 이해하고 위와 같이 수정해보았습니다.
피드백 해주실 때 $snackbar 를 가진 객체를 통해 snackbar.show(message, showtime) 형태로 호출하는 형태로 말씀해주셨었는데 제가 어떻게 구현해야할지 몰라서,,,😭 이해한 선에서 구현해봤습니다...! 이어서 피드백 주시면 감사하겠습니다 🙇🏻♀️
There was a problem hiding this comment.
class Snackbar {
constructor(element, time = 0) {
this.element = element
this.time = time
}
show(message) {
this.element.innerText = message
this.element.style.display = 'block'
}
hide() {
setTimout(() => {
this.element.style.display = 'none'
}, this.time)
}
}
const snackbar = new Snackbar($('.js-snackbar'))
snackbar.show("...")
위와 같은식을 말씀드렸던 것입니다.
|
수고 많으셨습니다. |
학습로그open API 사용법태그
|
자프님, 안녕하세요!
3단계로 돌아온 🙋🏻♀️ 하루(@365kim) 입니다.
이미 지하철 미션이 진행중인데 뒤늦게 유튜브 미션 리뷰요청드리게 된 점 양해말씀 구해봅니다,,, 🙏🙏
유튜브 강의실 미션
데모 페이지 바로가기
3단계 - 구현기능 목록
📌 리뷰반영 요약표
2단계 에서 코멘트 남겨주신 부분도 함께 반영해보았습니다! 🙇🏻♀️
확인해주시면 감사하겠습니다!
감사합니다 😆