Skip to content

[H팀 김홍석] 백엔드 API 과제 제출 - #4

Open
drghdtjr wants to merge 7 commits into
jinu0328:mainfrom
drghdtjr:main
Open

[H팀 김홍석] 백엔드 API 과제 제출#4
drghdtjr wants to merge 7 commits into
jinu0328:mainfrom
drghdtjr:main

Conversation

@drghdtjr

Copy link
Copy Markdown

No description provided.

@jinu0328 jinu0328 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

swagger 연동에 mysql을 docker-compose를 통해 세팅하시고 새로 연동까지 해주시고 과제 요구사항도 충실히 구현해주셔서 잘 보았습니다 👍
몇가지 피드백을 남겨두었으니 참고해주시면 좋을 것 같습니다. 😄
과제 고생 많으셨습니다!

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Swagger 연동까지 해주셨군요 좋습니다 👍

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

제네릭 타입을 활용한 커스텀 응답객체를 활용해서 응답의 일관성을 맞춰주신 것도 너무 좋습니다 👍


@Column
private LocalDateTime createdAt = LocalDateTime.now();

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

createdAt 필드의 기본값이 지정되어있지만 이 경우 JPA에서 엔티티를 재생성하거나 복원할 때 의도치 않게 다시 초기화 될 가능성이 있어보입니다. PrePersist 어노테이션을 붙여 초기값을 설정하면 더욱 안전할 것 같습니다 😄

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

응답 dto를 따로 구성해주신 만큼 엔티티 객체에 getter를 활용하는 것보다 dto 변환 메서드를 추가해주는 것이 엔티티의 안전성을 더 보장해줄 수 있는 방법이 될 것 같습니다.

ex)

public BoardResponse toDto() {
    return new BoardResponse(id, title, isDone)
}

.build();
boardRepository.save(updatedBoard);
}

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

업데이트 시 기존 객체의 id를 지닌 새로운 객체를 생성하는 방식으로 보입니다. 이 방식은 엔티티 객체를 수정할 때는 잘 활용되지는 않는 방식입니다. 데이터베이스의 ID 중복 오류를 발생시킬 수 있어 보입니다. Jpa의 자동 변경 감지 기능을 활용하여 기존 엔티티를 조회한 후 필요한 필드만 수정하는 방식으로 리팩터링하는 것이 좋을 것 같습니다 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants