-
Notifications
You must be signed in to change notification settings - Fork 6
과제 1-1 hongjm0912 과제제출 #6
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
base: master
Are you sure you want to change the base?
Conversation
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.
JWT 의존성은 왜 추가하셨나요😅 인증/인가도 다 해버리신줄 알고 당황했네요👍🏻
수고하셨습니다!🎉
@PatchMapping("/{idx}") | ||
public ResponseEntity<String> sujeong( | ||
@PathVariable Long idx, | ||
@RequestBody ArticleDTO dto) { | ||
articleService.sujeong(idx, dto); | ||
return ResponseEntity.ok("게시글이 수정되었습니다."); | ||
} | ||
|
||
@DeleteMapping("/{idx}") | ||
public ResponseEntity<String> deleteByTitle(@PathVariable Long idx) { | ||
articleService.sakjeByTitle(idx); | ||
return ResponseEntity.noContent().build(); | ||
} |
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.
메서드명 영어로 올바르게 통일해주세요
@ExceptionHandler(IllegalArgumentException.class) | ||
public ResponseEntity<String> handleNotFound(IllegalArgumentException ex) { | ||
return ResponseEntity.status(HttpStatus.NOT_FOUND).body(ex.getMessage()); | ||
} | ||
} |
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.
이렇게 핸들링하면 모든 IllegalArgumentException
을 핸들링하게 되서 예외 클래스를 따로 생성하는 등의 방식으로 하여 세부화하여 핸들링 해주시면 좋겠습니다!
post.setTitle(dto.getTitle()); | ||
post.setContent(dto.getContent()); |
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.
Setter(설정자) 사용은 지양하는게 좋습니다! Entity에서의 사용은 더더욱 지양해야하고요.
설정자의 사용은 개발자로 하여금 데이터를 빠르고 쉽게 수정가능 하도록 해주지만 다른 개발자로서 해당 행동이 왜 필요하고 최종적으로 어떤 목적으로 수정되는지 한번에 알기 어렵게 하고 외부에서 Entity가 의도치 않게 변경될 수 있습니다
public List<ArticleDTO> findList() { | ||
List<ArticleEntity> posts = articleRepository.findAll(); | ||
if (posts.isEmpty()) { | ||
return Collections.emptyList(); // 데이터가 없으면 200 OK + 빈 리스트([]) 반환 |
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.
따로 Empty를 감지하고, Collections.emptyList()를 반환한 이유가 무엇인가요?
ArticleEntity post = articleRepository.findById(idx) | ||
.orElseThrow(() -> new IllegalArgumentException("존재하지 않는 게시글입니다.")); | ||
|
||
return new ArticleDTO( |
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.
정말 필요한 수정 사항은 아니지만, Entity to DTO를 담당하는 코드를 함수로 분리하는것도 좋은방법일것 같습니다.
과제 마무우리