과제 1-1 boxinthechaos 과제제출 #1
Conversation
| @Getter | ||
| @Setter | ||
| @NoArgsConstructor | ||
| @ToString | ||
| public class boarddto { | ||
| private String title; | ||
| private String content; | ||
| } |
There was a problem hiding this comment.
Java 카멜케이스에 따라 이름을 boardDto로 하는게 올바를 것 같습니다
또한, @Setter 사용을 지양해주시고 builder 패턴이나, 생성자,Java 16 이상부터는 record 라는 문법을 활용할 수 있는데 그런 방식을 사용하여 주세요
| if(bordRespository.existsById(id)){ | ||
| bordRespository.deleteById(id); | ||
| return ResponseEntity.noContent().build(); | ||
| } | ||
| else{ | ||
| return ResponseEntity.notFound().build(); | ||
| } | ||
| } |
There was a problem hiding this comment.
이런 검증 로직은 Application 계층(Service)로 위임하는게 좋아보이고 무엇보다 Presentation 계층(Controller)에서 Persistence(Entity,Repository) 계층을 호출하는 것은 좋지 않습니다
2월20일 부터 진행될 과제 2-1에서 이와 관련된 법칙을 다시 복습할 것이긴 한데 참고해주세요
| private final service Service; | ||
| private final com.gsm._8th.class4.backed.task._1._1.global.Repositroy.bordRespository bordRespository; |
There was a problem hiding this comment.
FQCN 방식보단 Import 방식으로 하는게 좋아보입니다
| @PatchMapping("/{articlesid}") | ||
| public ResponseEntity<Article> updateArticles(@PathVariable Long id, @RequestBody Article article) { | ||
| return Service.updateArticles(id,article.getTitle(),article.getContent()); | ||
| } | ||
|
|
||
| @PostMapping("/articels") | ||
| public ResponseEntity<Article> creatArticles(@RequestBody Article article){ | ||
| return Service.creatArticles(article.getTitle(), article.getContent()); | ||
| } |
There was a problem hiding this comment.
Entity를 노출하는 설계는 전혀 좋지 않습니다.DTO(Data Transfer Object)를 이용하여 주세요
| public Article(String title, String content) { | ||
| this.title = title; | ||
| this.content = content; | ||
| } |
There was a problem hiding this comment.
생성자 보일러플레이트 코드가 Entity에 필요한지 부터 고민해보셔야 할 것 같습니다,좋은 코드에 가까운 설계라면 일단 생성자 자체가 Entity에는 필요가 없을 것 같은데 그거는 애플리케이션마다 요구사항이 다를 수 있으니 넘어가더라도 Lombok의 @AllArgsConstructor 어노테이션을 이용하면 보일러플레이트 코드를 줄일 수 있습니다
| if(bordrespository.existsById(id)){ | ||
| bordrespository.deleteById(id); | ||
| return ResponseEntity.noContent().build(); | ||
| } | ||
| else{ | ||
| return ResponseEntity.notFound().build(); | ||
| } | ||
| } |
There was a problem hiding this comment.
existsById로 SELETE 쿼리 발생 후 deleteById로 DELETE 쿼리는 불필요한 2중 쿼리 같은데 차라리 try-catch문을 이용하여 DELETE 쿼리에서 예외 발생 시 이를 캐치하여 NotFound를 반환하게 하는게 더 좋아 보입니다
| public ResponseEntity<Article> updateArticles(Long id, String title, String content){ | ||
| Article article = bordrespository.findById(id).orElse(null); | ||
| if(article != null){ | ||
| article.update(title,content); | ||
| return ResponseEntity.ok(article); | ||
| } | ||
| else { | ||
| return ResponseEntity.notFound().build(); | ||
| } | ||
| } |
There was a problem hiding this comment.
aricle 객체에 update 메서드 동작시키고 repository의 save 메서드 없이 그냥 반환값 작성해서 돌려보내는데 동작하나요?
| public ResponseEntity<Article> creatArticles(String title, String content){ | ||
| Article article = new Article(title,content); | ||
| Article savearticle = bordrespository.save(article); | ||
| return ResponseEntity.ok(savearticle); | ||
| } |
There was a problem hiding this comment.
Entity 객체를 직접 생성하는게 아니라 앞서 언급되었듯 DTO를 이용하여 DTO를 생성하든 한 다음에 Mapper 클래스나 메서드를 별도로 작성하여서 그를 이용하는게 좋을 듯 합니다
가능하다면 Persistence 계층 외부로 Entity가 노출되지 않는게 좋습니다
| @Service | ||
| @RequiredArgsConstructor | ||
| public class service { | ||
| private final bordRespository bordrespository; |
|
|
||
| @Service | ||
| @RequiredArgsConstructor | ||
| public class service { |
There was a problem hiding this comment.
클래스 이름은 현재 클래스가 무슨 역할인지 확실히 나타내는게 좋습니다.
역시 과제2-1에서 관련 법칙을 복습하며 리마인드 하겠지만...참고해주세요
일단 최대한 만들어 봤습니다