-
Notifications
You must be signed in to change notification settings - Fork 27
[이현성_BackEnd] 11주차 과제 제출합니다. #25
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.
고생하셨습니다!
현재 Controller에 비즈니스 로직이 다 들어가있는데 다음 과제 진행하면서 Service로의 분리를 해보면 좋을 것 같아요!
Controller <-> Service <-> Repository 의 구조를 공부하고 적용해주세요~!
| private final Map<Integer, Article> articles = new HashMap<>(); | ||
| private Integer articleId = 1; | ||
|
|
||
| @PostMapping("/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.
아래에는 /articles로 되어있는것도 있는데 경로를 통일해주면 좋을 것 같아요!
| } | ||
| } | ||
|
|
||
| @GetMapping("/articles") |
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.
이 컨트롤러 내에서 /article로 시작하는게 반복되고 있는데 @RequestMapping 에 대해 알아보고 적용해보면 좋을 것 같아요!
| @PostMapping("/article") | ||
| @ResponseBody | ||
| public ResponseEntity<?> createArticle(@RequestBody Map<String, String> body) { | ||
| try { |
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.
try catch 안에 넣는 방법 말고 예외가 발생하면 던지는 방식은 어떨까요?
| import java.time.LocalDateTime; | ||
| import java.util.*; | ||
|
|
||
| @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.
@RestController 에 대해 알아보면 좋을것 같아요!
반복되는 @ResponseBody 를 줄일 수 있을 것 같네용
| if (body.containsKey("title")) { | ||
| article.setTitle(body.get("title")); | ||
| } | ||
| if (body.containsKey("content")) { | ||
| article.setContent(body.get("content")); | ||
| } | ||
|
|
||
| article.setUpdatedAt(LocalDateTime.now()); |
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.
이걸 한번에 해주는 방법은 어떤가요??
| return map; | ||
| } | ||
| } | ||
| } No newline at end of file |
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.
EOF에 대해서 알아면 좋을 것 같아요!
참고 : https://velog.io/@junho5336/No-newline-at-end-of-file
| this.updatedAt = LocalDateTime.now(); | ||
| } | ||
|
|
||
| public Article(Integer id, Integer authorId, Integer boardId, String title, String content, LocalDateTime createdAt, LocalDateTime updatedAt) { |
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.
가독성을 위해 개행을 해줘도 좋을 것 같아요
| return id; | ||
| } | ||
|
|
||
| public void setId(Integer 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.
setter의 단점은 무엇일까요?
|
7주차 과제 제출입니다. |
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.
코드가 깔끔하네요 👍
리뷰에 답변 남겨주시고 pr 제목도 8주차로 수정해주세요!
고생하셨습니다!
| @RestController | ||
| @RequestMapping("/articles") | ||
| public class ArticleController { | ||
| private final PostService postService; |
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.
Article 관련인데 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.
Article관련인데ArticleService가 아닌 이유가 있을까요?
html에 보여줄때 post html을 같이 사용해서 따로 분리하지 않고 post에 같이 넣어둔건데 분리하겠습니다!
| } | ||
|
|
||
| @GetMapping | ||
| public List<Article> getArticlesByBoard(@RequestParam("boardId") int 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.
이 부분은ResponseEntity 적용이 안되어 있는 이유가 있나요??
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.
DTO 도 알아보고 적용하면 좋을 것 같아요!
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.
DTO도 알아보고 적용하면 좋을 것 같아요!
이 부분에 관해서 살짝 이해가 안되는 부분이 있던데, 모델을 작성하는것과 DTO의 큰 차이가 뭔지 잘 모르겠습니다.
| @Autowired | ||
| public MemberController(MemberService memberService) { | ||
| this.memberService = memberService; | ||
| } |
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 RowMapper<Article> articleRowMapper() { | ||
| return (rs, rowNum) -> { | ||
| Article dto = new Article(); | ||
| dto.setId(rs.getInt("id")); | ||
| dto.setAuthorId(rs.getInt("author_id")); | ||
| dto.setBoardId(rs.getInt("board_id")); | ||
| dto.setTitle(rs.getString("title")); | ||
| dto.setContent(rs.getString("content")); | ||
| dto.setCreatedAt(rs.getTimestamp("created_date").toLocalDateTime()); | ||
| dto.setUpdatedAt(rs.getTimestamp("modified_date").toLocalDateTime()); | ||
| return dto; | ||
| }; | ||
| } |
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.
클래스 최상단으로 올리면 좋을 것 같아요!
| import org.springframework.web.bind.annotation.*; | ||
|
|
||
| @RestControllerAdvice | ||
| public class GlobalExceptionHandler { |
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.
👍
| } | ||
|
|
||
| @Transactional | ||
| public void updateMember(int id, Member memberUpdateData) { |
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.
메서드가 너무 길다고 느껴지는데 어떻게 해결할 수 있을까요?
| this.memberDao = memberDao; | ||
| } | ||
|
|
||
| @Transactional(readOnly = true) |
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.
클래스 단에 붙여놓으면 이렇게 메서드마다 붙이지 않아도 됩니다!
No description provided.