-
Notifications
You must be signed in to change notification settings - Fork 2
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
Yee #2
base: master
Are you sure you want to change the base?
Yee #2
Conversation
삭제기능 깜빡한거 추가했어요 |
} | ||
|
||
public void completeTodo(ChildTodo childTodo) { | ||
// todo: 입력 받은 childTodo의 완료 처리와 동시에 parentTodo의 모든 details 내에 있는 todo가 완료 되었으면 parentTodo도 완료 처리. | ||
// 입력 받은 childTodo의 완료 처리와 동시에 parentTodo의 모든 details 내에 있는 todo가 완료 되었으면 parentTodo도 완료 처리. | ||
int index=getDetailsIndexbyChildTodo(childTodo); |
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.
commit 하기 전에 ctrl + alt + l 을 눌러서 정렬을 하던지, 아니면 intelliJ의 commit 할 때 reformat 해주는 기능을 이용해서 포맷을 자동으로 맞춰주도록 하자... 포맷은 생각보다 가독성에 많은 영향을 줌.
@@ -8,6 +8,16 @@ | |||
*/ | |||
public class TodoResponse { | |||
|
|||
public TodoResponse(List<TodoInquiryDto> todoList){ |
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.
보통 private field 아래에 constructor, method 순서로 작성하는 것이 일반적임.
이유는 이 클래스가 가지는 변수를 선언하는 것이기 때문에 마치 C에서 변수 먼저 다 선언하고 처리하는 것과 같은 것.
고로 아래로 내려보내는 것이 좋을 듯함.
// todo | ||
return null; | ||
// | ||
List<TodoInquiryDto> todoList=Collections.emptyList(); |
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.
애초에 ParentTodo를 받아서 처리하는 함수를 짰으면... ParentTodo만 받아주도록 stream으로 할 수 있지 않았을까? add를 매번 해주려고 하면 성능도 너무 떨어질 것 같은데,
List<TodoInquiryDto> todoList = this.todoEntities.stream()
.filter(ParentTodo.class::instanceOf)
.map(this::createTodoInquiryDtoFromParentTodo)
.collect(Collectors.toList());
이런 식이면 한번에 변수 선언도 따로 할 필요 없고 저 함수를 불러서 변환한다는게 딱 보이고 깔끔할듯.
id.add(parentTodo.getId()); | ||
String title = parentTodo.getTitle(); | ||
String content = parentTodo.getContent(); | ||
for (ChildTodo childTodo : parentTodo.getDetails()) { |
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.
여기도 그냥 stream 돌리면 깔끔하게 add 없이 돌아가서 더 좋을것.
// done | ||
for (TodoUpdateDto todoUpdateDto : request.getTodoUpdateList()) { | ||
boolean isTodoInTheList = this.todoEntities.stream() | ||
.anyMatch(todo -> todo.getId() == todoUpdateDto.getId()); |
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.
intelliJ가 여기에 노란 highlighter 안보여주더냐... equals 사용하라고 할 거 같은데;; todo.getId().equals(todoUpdateDto.getId()) 로 바꾸라고 시킬듯.
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.
추가로, todoEntity에는 없는데 들어오면 그냥 새로 만들어버리겠네 이건... Id값 다르게해서 만들텐데 그게 맞는지는 모르겠다. 보통은 이런 경우에 아예 안만들어주도록 함.
이유는 첫째로 DB에 데이터 넣어주는 것은 최대한 보수적으로 해야하기 때문이고,
둘째로 데이터가 Id만 잘못들어온 요청일 수 있는데 무조건 데이터를 만들기보다는 잘 안되었을 때 그것을 프론트가 다시 요청해서 정확하게 수정하면 굳이 DB 건드릴 것도 없이 정상적인 데이터 처리를 해줄 수 있게 되기 때문임.
DB는 어디까지나 DB를 직접 건드릴 일 없게 해주는 것이 최고이기 때문에, 이런 방식으로 진행함.
따라서 삭제 요청을 먼저 진행하고나서, 가지고 들어온 것 저장 요청들 중에 id가 있으면 entity에서 id가 같은 것을 찾아서 처리하고, 없으면 에러를 throw 하도록 해주는 것이 맞고
id가 없을 때만 새로 만들어주는 것이 맞음.
@@ -16,10 +16,16 @@ public ParentTodo getParent() { | |||
} | |||
|
|||
public void setParentTodo(ParentTodo parent) { | |||
// todo: parentTodo를 지정하고, parentTodo의 details에 이 childTodo가 없으면 details에도 추가. | |||
// parentTodo를 지정하고, parentTodo의 details에 이 childTodo가 없으면 details에도 추가. | |||
this.parent=parent; |
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.
reformat
새로운 todo른 만드는 조건을 말씀하신 대로 id가 존재하지 않을 때로 바꿨습니다. |
이리저리 추가했어요.
적은지 2주나 지나서 기억은 잘 안나네요