-
Notifications
You must be signed in to change notification settings - Fork 13
[Spring Core] 이동훈 과제 제출합니다. #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: main
Are you sure you want to change the base?
Conversation
daheeParkk
left a comment
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.
수고하셨습니다! 👍🏻
| public ResponseEntity<ArticleResponse> crateArticle( | ||
| @RequestBody ArticleCreateRequest request | ||
| @Valid @RequestBody ArticleCreateRequest request | ||
| ) { | ||
| if (boardService.getBoards().stream() | ||
| .noneMatch(board -> board.id().equals(request.boardId())) || | ||
| memberService.getAll().stream() | ||
| .noneMatch(member -> member.id().equals(request.authorId()))) | ||
| throw new ExceptionGenerator(StatusEnum.CREATE_NOT_PRESENT_BOARD); | ||
|
|
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.
예외 처리를 모두 controller에서 하고 있네요!
위와 같은 로직을 service에서 한다면 코드를 재활용 할 수 있지 않을까요?
ArticleController의 crateArticle()과 updateArticle() 에서도 중복되는 부분이 보입니다.
| out/ | ||
| !**/src/main/**/out/ | ||
| !**/src/test/**/out/ | ||
| *.yml |
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.
적용 중이던 .gitignore에 추가로 작성하면 적용이 안됩니다! 방법을 찾아보세요..!
daheeParkk
left a comment
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.
수고하셨습니다!
| @GetMapping("/articles") | ||
| public ResponseEntity<List<ArticleResponse>> getArticles( | ||
| @RequestParam Long boardId | ||
| @RequestParam Long boardId | ||
| ) { | ||
| List<ArticleResponse> response = articleService.getByBoardId(boardId); | ||
|
|
||
| articleValidate.validateResponseIsEmpty(response); | ||
|
|
||
| return ResponseEntity.ok(response); | ||
| } |
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.
controller에 articleValidate.validateResponseIsEmpty(response);와 같이 검증 메서드를 사용하는 로직이 있네요!
검증해야 할 게 바뀌거나 추가될 때마다 controller에서 수정을 해야 하는 문제가 생길 것 같아요
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.
그러면 컨트롤러에서는 validateResponseIsEmpty 같은 메소드를 사용하는 것이 아니라 validateCreateArticle, validateUpdateArticle과 같은 메소드를 호출하게 만들고 그 메소드 속에서 validateResponseIsEmpty를 호출하게 만들면 나중에 검증 로직에 수정사항이 있을 때 컨트롤러에서 수정할 일이 없을 것 같습니다! 위와 같은 방식으로 수정해두겠습니다.
| private final MemberRepository memberRepository; | ||
| private final BoardRepository boardRepository; | ||
|
|
||
| public ArticleService( |
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.
예외처리도 필요할 것 같아요
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.
제가 잘못 이해한걸수도 있지만 예외 처리는 src/main/java/com/example/demo/exception/ExceptHandler.java 에서 처리하도록 만들었습니다.
| return modifiedAt; | ||
| } | ||
|
|
||
| public void setBoardId(Long boardId) { |
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.
세터를 지양하는 이유에 대해 찾아보면 좋을 것 같습니다!
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.
넵! 찾아보고 다음 보고서에 정리해서 작성해두겠습니다!!
8주차 실습 과제 제출합니다.