Conversation
- 스켈레톤 코드 작성
- 스켈레톤 코드 작성
- data 입력
- data배열 exports
- inputCommand 함수 생성 - input data 유효성 검사
- excuteTodo 함수 생성 - action 파라미터 유효성 검사
- 수도코드 작성 - exports
- getStatusList 메소드 추가 - printAll 메소드 추가 - show 메소드 추가
- printList 메소드 생성
- getId메소드 추가
- add 메소드 추가 - prompt 재출력 코드 추가
- readline 파라미터 제거
- delete 메소드 추가
- 생성자 추가 - 프롬프트 출력 오류 수정
- 인터페이스 구조 변경
- update 메소드 추가
- printError 메소드 생성 - 버그 수정
- 에러메세지 모듈 생성
- 에러메세지 출력 버그 수정
- statusList 출력 방식 변경
- 태그 에러케이스 제거
- try catch throw 활용 - add 메소드 수정
- try catch throw 활용 - errorMessage require
- 코드 중복제거
crongro
left a comment
There was a problem hiding this comment.
수고하셨네요. 코드가 간결한 편이라 보기 좋습니다.
todo를 class를 써서 아쉽지만, 5-3스텝에서 prototype으로 마이그레이션 하시면 됩니다 ^^
리뷰 남긴 부분 참고해서 먼저 수정해보시고, 끝나시면 5-3진행하세요~!
| input: process.stdin, | ||
| output: process.stdout | ||
| }); | ||
| const myTodo = new todo(rl); |
There was a problem hiding this comment.
생성자역할의 함수라면 대문자로 시작하는 게 좋아요. Todo이렇게요. 이게 일반적이라서요.
나중에 class이름도 마찬가지고요.
| rl.on('line', input => { | ||
| const inputArray = input.split('$'); | ||
| if (inputArray.length === 1) { | ||
| return myTodo.printError('COMMAND_ERROR'); |
There was a problem hiding this comment.
length가 0이면, 또는 length가 1000이면 에러가 아닌가요?
|
|
||
| const excuteTodo = (action, condition) => { | ||
| const regExp = /^show$|^add$|^delete$|^update$/; | ||
| const matchRegExp = action.match(regExp); |
There was a problem hiding this comment.
정규표현식은 간단한 부분부터 이렇게 적용하면서 공부하는 게 좋죠.
| const regExp = /^show$|^add$|^delete$|^update$/; | ||
| const matchRegExp = action.match(regExp); | ||
|
|
||
| try { |
There was a problem hiding this comment.
throw와 try/catch의 관계 등을 잘 이해하고 적용하실 수 있어야 할텐데요. 여기 적용한것은 잘 이해하신 거 같네요.
| @@ -0,0 +1,5 @@ | |||
| module.exports = { | |||
There was a problem hiding this comment.
에러메시지를 객체리터럴로 분리한 거 좋아요. 이런코드는 늘 이렇게 분리해버리세요.
| const errorMessage = module.require('./errorMessage.js'); | ||
| module.require('date-utils'); | ||
|
|
||
| module.exports = class todo { |
There was a problem hiding this comment.
prototype을 안썼군요.
5-3에서는 지금 class코드를 반대로 prototype 패턴으로 바꾸셔야합니다. ^^
| todoList.push(newData); | ||
| console.log(`${newData.name} 1개가 추가되었습니다. (id : ${newData.id})`); | ||
| setTimeout(() => { | ||
| this.printAll(); |
There was a problem hiding this comment.
arrow함수를 안쓰면 여기의 this는 무엇이 될까요? (중요함~!)
한번 실험하고, 이유도 살펴보면 좋겠네요.
There was a problem hiding this comment.
arrow함수를 안쓰면 this는 전역객체를 바라보게 됩니다.
arrow함수를 안쓰고 원래 의도대로 하려면 아래처럼 사용하면 됩니다.
add(name, tag) {
const that = this;
setTimeout(function() {
that.printAll();
}, 1000);
}
| } | ||
|
|
||
| getStatusList() { | ||
| const listOfStatus = todoList.reduce((acc, cur) => { |
There was a problem hiding this comment.
todoList를 이렇게 require로 접근 받아서 호출해서 쓸 수도 있지만,
todo class의 생성자에서 받아서 this.todoList=todoList; 이렇게 접근하는 것도 방법이죠!
뭐가 더 좋을까요?
| setTimeout(() => { | ||
| this.printAll(); | ||
| }, 1000); | ||
| }, 3000); |
There was a problem hiding this comment.
매직넘버는 모두 언제나 늘 없애야 코드가 변경에 용이한 코드가 됩니다.
| return Number(id) === element.id; | ||
| } | ||
| }); | ||
| if (targetData[0] === undefined) { |
There was a problem hiding this comment.
지금 상황은 undefined 가 명확해! 그게 아닌 다른 타입은 모두 정상이야!! 라고 할 수 있다고 치고,,
타입비교로 에러검증을 할때는, 좀더 정확히 신중할 필요도 있습니다.
class를 이용해서 작성했습니다.
리뷰 부탁드립니다.