Step5-3 객체지향프로그래밍 왕민 & 묵은지#123
Conversation
- 생성자 함수 이름 변경
- 에러발생 조건 추가
- todoList 접근방식 변경
- 매직넘버 -> 상수로 변경
- log class 생성 - Log.js 수도코드 작성
- addLog 메소드 추가
- require errorMessage in app.js
- add UNDO_ERROR message - add REDO_ERROR message
- undo method 추가 - alterData method 추가
- addLog 메소드 호출
- undo 메소드 추가 - redo 메소드 추가 - 정규표현식 조건 추가
- redo 메소드 추가 - inputarray 조건 변경 - 객체 참조 변수 -> object.assign()
- 에러발생 조건 추가
- 에러발생 조건 추가
- gitignore 파일 생성
- update 메소드 에러 케이스 추가
- undo/redo 에러 메세지 추가
- todoList data 조건 추가
- 데이터 오류 수정
- 에러메세지 수정
- class -> prototype 변경
crongro
left a comment
There was a problem hiding this comment.
queue를 사용해서 잘 구현했습니다.
큰 맥락에서는 좋고요,
리뷰드린 내용을 조금더 수정해보시면 좋겠네요.
oop/todo.js
Outdated
| } | ||
| todoList[index].status = status; | ||
| Todo.prototype.undo = function() { | ||
| todoLog.undo(); |
There was a problem hiding this comment.
undo/redo 가 있는 Log에서 todolist를 직접 표현해서 쓰지 않고,
여기와 같이 undo/redo 메서드를 부를때 todoList값을 인자로 넣어주는 게 어때요?
oop/todo.js
Outdated
| }; | ||
| this.todoList.push(newData); | ||
| const prevData = {}; | ||
| Object.assign(prevData, newData); |
There was a problem hiding this comment.
두 줄을, 한 줄로 표현할 수 있겠네요.
var prevData = Object.assign({}, newData);
| prevData.status = '삭제'; | ||
| console.log(`${newData.name} 1개가 추가되었습니다. (id : ${newData.id})`); | ||
| todoLog.addLog('add', prevData, newData, this.todoList.length - 1); | ||
| setTimeout(() => { |
There was a problem hiding this comment.
add메서드가 여러개를 하는데, 좀 나누자면 예를들어서 이렇게도 가능하죠.
add를 해서 데이터를 추가하고, 추가한 결과를 받은후에, addLog수행하고 print수행하기.
oop/todo.js
Outdated
|
|
||
| console.log(`${deletingName}가 ${this.todoList[index].status}에서 삭제됐습니다.`); | ||
| const nextData = {}; | ||
| Object.assign(nextData, this.todoList[index]); |
oop/Log.js
Outdated
| }; | ||
|
|
||
| Log.prototype.addLog = function(action, prevData, nextData, todoListIndex) { | ||
| if (this.queue.length > 4) { |
There was a problem hiding this comment.
변경될 소지가 있으니 4라는 숫자 없애보기. (다른곳으로 옮겨서 표현해야겠죠)
oop/Log.js
Outdated
| const todoListIndex = this.queue[this.index].todoListIndex; | ||
|
|
||
| if (action === 'add') { | ||
| this.alterData(todoListIndex, 0, nextData); |
There was a problem hiding this comment.
인자3개를 넘겨주는게
인자를 객체 하나를 사용하는 방법으로 수정해보세요.
- reduce code
- change magicNumber to variable - reduce unnecessary codes
- add validCheck.js - refacoring program structure
|
전체적인 구조 리팩토링 완료했습니다. |
crongro
left a comment
There was a problem hiding this comment.
undo/redo 과정에서 실제 todolist의 데이터 상태가 변경되는 건가요?
oop/log.js
Outdated
| @@ -0,0 +1,65 @@ | |||
| module.exports = Log = function() { | |||
| (this.queue = []), (this.index = -1), (this.undoLimit = 3); | |||
There was a problem hiding this comment.
이렇게 표현하는 게 전 좀 낯서네요
(this.queue = []), (this.index = -1), (this.undoLimit = 3);
-1,3 도 인자로 받으면 더 좋고요. default parameter도 활용해서 기본값도 셋팅할 수 있고요.
There was a problem hiding this comment.
네! 실제 todolist 데이터 변경됩니다.
구조 변경하면서 발생한 버그 수정했고 default parameter 사용했습니다.
(this.queue = []), (this.index = -1), (this.undoLimit = 3);
이표현은 프리티어가 제멋대로...
아래 표현이 의도한 표현입니다.
this.queue = [],
this.index = -1,
this.undoLimit = 3
- undo/redo 메소드 리턴 버그 수정 - 생성자함수 디폴트 파라미터 설정
- 삭제될 데이터 버그 수정
- excuteTodo 메소드 버그 수정
crongro
left a comment
There was a problem hiding this comment.
수정하신 거 확인했어요.
수고하셨어요! 승인합니다.
| module.exports = Log = function() { | ||
| (this.queue = []), (this.index = -1), (this.undoLimit = 3); | ||
| module.exports = Log = function(undoLimit = 3, defaultIndex = -1) { | ||
| (this.queue = []), (this.index = defaultIndex), (this.undoLimit = undoLimit); |
There was a problem hiding this comment.
괄호가 좀 불필요해보이긴 하는데요.
취향으로 인정할게요 ㅎ
리뷰 부탁드립니다.