Conversation
BaeJinho4028
left a comment
BaeJinho4028
left a comment
There was a problem hiding this comment.
전체적인 컨벤션한번씩 체킹해주시면 좋을 것 같습니다.
코드 자체는 컴팩트하고 깔끔하네요 👍
그리고 커밋명에 몇주차인지 써주시면 좋을 것 같아요:)
| public ResponseEntity<Article> createArticle(@RequestBody Article article) { | ||
| Article saved = articleService.createArticle( | ||
| article.getAuthorId(), | ||
| article.getBoardId(), | ||
| article.getTitle(), | ||
| article.getContent() | ||
| ); |
There was a problem hiding this comment.
모델을 바로 RequestBody로 받으셨군요.
이것에 대한 장단점은 무엇이 있을까요?
그리고 articleService.createArticle()을 호출할때도 Article을 넘겨줘도 괜찮지 않을까요?
There was a problem hiding this comment.
1.RequestBody를 쓰는게 가장 코드를 쉽게 쓸 수 있어서 사용하였습니다. 단점을 찾아보니 추후에 보안 문제가 생길 수 있다하고 결합도가 증가해서 유지보수가 어려워진다는 단점이 있었습니다
2. 그런 것 같습니다! 엔티티 내용 옮기는데만 신경쓰다보니 그대로 넘길 생각을 못했습니다. 이번 10주차 리팩토링에서는 DTO를 추가하면서 DTO를 통해 article을 생성하도록 수정했습니다
| import java.util.Map; | ||
|
|
||
| @RestControllerAdvice | ||
| public class GlobalExceptionHandler { |
| public void setName(String name) { | ||
| this.name = name; | ||
| } | ||
|
|
||
| public void setEmail(String email) { | ||
| this.email = email; | ||
| } | ||
|
|
||
| public void setPassword(String password) { | ||
| this.password = password; | ||
| } |
There was a problem hiding this comment.
setter는 지양하는 것이 좋은데, 그 이유와 대체 방법들을 알아두시면 좋을 것 같습니다.
| public Long getId() { return id; } | ||
| public Long getAuthorId() { return authorId; } | ||
| public Long getBoardId() { return boardId; } | ||
| public String getTitle() { return title; } | ||
| public String getContent() { return content; } | ||
| public LocalDateTime getCreatedDate() { return createdDate; } | ||
| public LocalDateTime getModifiedDate() { return modifiedDate; } | ||
|
|
||
| public void setTitle(String title) { this.title = title; } | ||
| public void setContent(String content) { this.content = content; } |
There was a problem hiding this comment.
다른 것들은 들여쓰기가 되어있는데, 여기 함수 스코프들은 들여쓰기가 안되어있네요. 통일이 되면 좋을 것 같습니다.
|
10주차 과제 진행하였습니다 피드백 받은대로 코드 스타일을 통일하였고 set함수를 지양하는 방식으로 리팩토링해보았습니다. DTO패키지를 추가했습니다 |
|
11주차 과제 진행하였습니다. 세션 방식을 통한 인증을 추가하였습니다. |
BaeJinho4028
left a comment
BaeJinho4028
left a comment
There was a problem hiding this comment.
리뷰를 조금 늦게 달게되어 죄송합니다.
그래도 코드는 깔끔하게 잘 짜주신 것 같습니다.
손수 손으로 코드를 짜는 것과 AI 활용을 잘 하는 것 둘 다 잘 챙기셨으면 좋겠습니다.
회고 화이팅하시고, 회고때 질문이나 고민 있으시면 언제든 말씀해주세요 👍
| } | ||
|
|
||
| @GetMapping | ||
| public ResponseEntity<List<Member>> getAllMembers() { |
There was a problem hiding this comment.
DB에서 쓰는 함수명을 외부에서는 안쓰는 것이 좋긴 합니다.
| Member member = memberRepository.findByEmail(request.getEmail()) | ||
| .orElseThrow(() -> new IllegalArgumentException("존재하지 않는 이메일입니다.")); | ||
|
|
||
| if (!member.getPassword().equals(request.getPassword())) { |
There was a problem hiding this comment.
BCryptPasswordEncoder를 사용해서
encoder.matches(rawPassword, encodedPassword);
로 사용하시는 것을 알아두시면 좋을 것 같습니다.
| public Article findArticle(Long id) { | ||
| return articleRepository.findById(id); | ||
| return articleRepository.findById(id) | ||
| .orElseThrow(() -> new IllegalArgumentException("게시물을 찾을 수 없습니다. id=" + id)); |
There was a problem hiding this comment.
404 NotFound이기 때문에 IllegalArgumentException보다는 다른 예외케이스를 사용하시는 것이 좋습니다.
| return article; | ||
| } | ||
|
|
||
| @Transactional(readOnly = true) |
There was a problem hiding this comment.
@transactional(readOnly=true)를 클래스에 걸고 CUD에 따로 다는 방식이 조금 더 범용적이더라구요.
| article.changeTitle(title); | ||
| article.changeContent(content); |
2 participants
스프링 부트 과제를 수행했습니다
최대한 공식문서를 활용해서 정보를 얻으려 했으나 공식문서는 예시보다 어노테이션 종류나 역할만 정리된 정보가 많아서 블로그나 티스토리를 많이 참고하게 되었습니다..
단순히 출력반환하는 코드를 작성했다가 그게 아니라는 걸 알고 수정을 많이 했는데
잘못된 코드 완성 >>커밋 이 아니라
잘못된 코드 작성>>포스트맨 실행이 전혀 안 됨/원하던 출력값 아님 >>수정>> 커밋
하다보니 수정하는 과정의 커밋을 날려먹었습니다 다음부터는 더 상세하게 커밋해보겠습니다.