-
Notifications
You must be signed in to change notification settings - Fork 13
[Spring Core] 강인화 과제 제출합니다. #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
base: main
Are you sure you want to change the base?
Conversation
dradnats1012
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 |
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.
tab이 한번 더 된것 같아요
ctrl + alt + L 하면 라인정리 깔끔하게 해주니까 참고하세용
맥이면 command + option + L입니다
| import java.net.URI; | ||
| import java.util.List; | ||
|
|
||
| @RestController |
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.
지금 공통적으로 "/articles"가 있는데 @RequestMapping("/articles") 를 해놓으면 해당하는 클래스에 공통적으로 적용돼서 뺄수 있습니다!
| import java.util.List; | ||
|
|
||
| @RestController | ||
| public class BoardController { |
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.
여기도 @RequestMapping 적용해보세요!
| boolean isNullExistence = request.name() == null | ||
| || request.email() == null | ||
| || request.password() == null; |
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.
그동안 일정이 바빴어서 지금 확인했습니다 ㅠ
피드백 주신 사항 전부 커밋해봤습니다!
| } | ||
| if (boardService.getBoards() | ||
| .stream() | ||
| .noneMatch(res -> res.id().equals(request.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.
이렇게 메소드 체이닝으로 길어진 구문은 보통 한줄로 적는지, 메서드 마다 들여쓰기를 하는지 궁금합니다!
dradnats1012
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.
고생하셨습니다!
|
|
||
| List<Board> findAll(); | ||
|
|
||
| Board findById(Long id); |
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.
findById()의 문제점이 뭐가 있을까요?
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 Long boardId; | ||
| @ManyToOne | ||
| @JoinColumn(name = "author_id") | ||
| private Member author; |
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.
적용했습니다! 어노테이션이랑 겹쳐서 필드간 구분이 어려웠는데 구별하기 더 용이해진 것 같습니다.
| Board board, | ||
| String title, | ||
| String content, | ||
| LocalDateTime createdAt, |
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.
ctrl + alt + L 누르면 한번에 정렬되는 단축키가 있어요!
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.
적용했습니다!
| request.title(), | ||
| request.description() | ||
| ); | ||
| Article saved = articleRepository.save(article); |
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.
saved 말고 좀더 좋은 변수명은 없을까요??
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.
더 구체적으로 createdArticle, savedArticle과 같은 것이 더 좋을 것 같습니다.
스프링 부트에서 예외처리 하는 방법과 과정에 대해 학습하고 실습해볼 수 있었습니다.