Skip to content

과제 1-1 boxinthechaos 과제제출 #1

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ version = '0.0.1-SNAPSHOT'

java {
toolchain {
languageVersion = JavaLanguageVersion.of(21)
languageVersion = JavaLanguageVersion.of(23)
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package com.gsm._8th.class4.backed.task._1._1.global.BoardController;

import com.gsm._8th.class4.backed.task._1._1.global.entity.Article;
import com.gsm._8th.class4.backed.task._1._1.global.service.service;
import lombok.RequiredArgsConstructor;
import org.springframework.http.ResponseEntity;
import org.springframework.stereotype.Controller;
import org.springframework.web.bind.annotation.*;

import java.util.List;

@Controller
@RequiredArgsConstructor
@RequestMapping("/articles")
public class boardcontroller {

private final service Service;
private final com.gsm._8th.class4.backed.task._1._1.global.Repositroy.bordRespository bordRespository;
Comment on lines +17 to +18
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FQCN 방식보단 Import 방식으로 하는게 좋아보입니다


//게시물 조회
@GetMapping("/articles")
public ResponseEntity<List<Article>> getArticles(){
return Service.getArticles();
}

//특정 게시물 조회
@GetMapping("/{articlesid}")
public ResponseEntity<Article> getArticlesById(@PathVariable Long id){
return Service.getArticlesById(id);
}

//게시물 삭제
@DeleteMapping("/{articlesid}")
public ResponseEntity<Article> deleteArticle(@PathVariable Long id){
if(bordRespository.existsById(id)){
bordRespository.deleteById(id);
return ResponseEntity.noContent().build();
}
else{
return ResponseEntity.notFound().build();
}
}
Comment on lines +35 to +42
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이런 검증 로직은 Application 계층(Service)로 위임하는게 좋아보이고 무엇보다 Presentation 계층(Controller)에서 Persistence(Entity,Repository) 계층을 호출하는 것은 좋지 않습니다

2월20일 부터 진행될 과제 2-1에서 이와 관련된 법칙을 다시 복습할 것이긴 한데 참고해주세요


//게시물 수정
@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());
}
Comment on lines +45 to +53
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Entity를 노출하는 설계는 전혀 좋지 않습니다.DTO(Data Transfer Object)를 이용하여 주세요

}

Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package com.gsm._8th.class4.backed.task._1._1.global.BoardDTO;

import lombok.Getter;
import lombok.NoArgsConstructor;
import lombok.Setter;
import lombok.ToString;

@Getter
@Setter
@NoArgsConstructor
@ToString
public class boarddto {
private String title;
private String content;
}
Comment on lines +8 to +15
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Java 카멜케이스에 따라 이름을 boardDto로 하는게 올바를 것 같습니다

또한, @Setter 사용을 지양해주시고 builder 패턴이나, 생성자,Java 16 이상부터는 record 라는 문법을 활용할 수 있는데 그런 방식을 사용하여 주세요

Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package com.gsm._8th.class4.backed.task._1._1.global.Repositroy;

import com.gsm._8th.class4.backed.task._1._1.global.entity.Article;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.stereotype.Repository;

@Repository
public interface bordRespository extends JpaRepository<Article, Long> {
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
/**
* 과제 정보를 출력하는 클래스에 적용되는 어노테이션
*
* @Componset 어노테이션을 사용하여 빈으로 등록된 클래스에 적용
* @Component 어노테이션을 사용하여 빈으로 등록된 클래스에 적용
* @Slf4j 어노테이션을 사용하여 로깅을 위한 Logger 객체를 생성
* 어노테이션은 런타임에도 유지되어야 하므로 @Retention 어노테이션을 사용하여 런타임에도 유지되도록 함
*/
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package com.gsm._8th.class4.backed.task._1._1.global.entity;

import jakarta.persistence.Column;
import jakarta.persistence.Entity;
import jakarta.persistence.Table;
import lombok.Getter;
import lombok.NoArgsConstructor;

@Entity
@Getter
@Table(name = "articles")
@NoArgsConstructor
public class Article extends BaseIdxEntity {

@Column(nullable = false, length = 255)
private String title;

@Column(nullable = false, length = 255)
private String content;

public Article(String title, String content) {
this.title = title;
this.content = content;
}
Comment on lines +21 to +24
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

생성자 보일러플레이트 코드가 Entity에 필요한지 부터 고민해보셔야 할 것 같습니다,좋은 코드에 가까운 설계라면 일단 생성자 자체가 Entity에는 필요가 없을 것 같은데 그거는 애플리케이션마다 요구사항이 다를 수 있으니 넘어가더라도 Lombok의 @AllArgsConstructor 어노테이션을 이용하면 보일러플레이트 코드를 줄일 수 있습니다


public void update(String title, String content) {
this.title = title;
this.content = content;
}
}
Comment on lines +1 to +30
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

설정자(Setter) 사용 안하신거는 잘하셨는데 VO나 DTO도 아니고 Entity에 생성자를 이용한 객체 생성보다는 Builder를 이용하는게 좀 더 Spring의 관례에 알맞는거 같습니다

Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,10 @@ abstract class BaseIdxEntity {
@GeneratedValue(strategy = GenerationType.IDENTITY)
@Column(name = "idx", nullable = false, updatable = false, insertable = false, unique = true)
protected Long idx;

@Column(nullable = false, name = "created_at")
protected LocalDateTime createdAt;

@Column(nullable = false, name = "updated_at")
protected LocalDateTime updatedAt;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package com.gsm._8th.class4.backed.task._1._1.global.service;

import com.gsm._8th.class4.backed.task._1._1.global.Repositroy.bordRespository;
import com.gsm._8th.class4.backed.task._1._1.global.entity.Article;
import lombok.RequiredArgsConstructor;
import org.springframework.http.ResponseEntity;
import org.springframework.stereotype.Service;
import java.util.List;
import java.util.Optional;

@Service
@RequiredArgsConstructor
public class service {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

클래스 이름은 현재 클래스가 무슨 역할인지 확실히 나타내는게 좋습니다.
역시 과제2-1에서 관련 법칙을 복습하며 리마인드 하겠지만...참고해주세요

private final bordRespository bordrespository;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오타 있네요😅


//게시물 조회
public ResponseEntity<List<Article>> getArticles() {
List<Article> articles = bordrespository.findAll();
return ResponseEntity.ok(articles);
}

//특정 게시물 조회
public ResponseEntity<Article> getArticlesById(Long id){
Article article = bordrespository.findById(id).orElse(null);
if (article != null){
return ResponseEntity.ok(article);
}
else {
return ResponseEntity.notFound().build();
}
}

//게시물 생성
public ResponseEntity<Article> creatArticles(String title, String content){
Article article = new Article(title,content);
Article savearticle = bordrespository.save(article);
return ResponseEntity.ok(savearticle);
}
Comment on lines +34 to +38
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Entity 객체를 직접 생성하는게 아니라 앞서 언급되었듯 DTO를 이용하여 DTO를 생성하든 한 다음에 Mapper 클래스나 메서드를 별도로 작성하여서 그를 이용하는게 좋을 듯 합니다
가능하다면 Persistence 계층 외부로 Entity가 노출되지 않는게 좋습니다


//특정 게시물 수정(업데이트)
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();
}
}
Comment on lines +41 to +50
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aricle 객체에 update 메서드 동작시키고 repository의 save 메서드 없이 그냥 반환값 작성해서 돌려보내는데 동작하나요?


//특정 게시물 삭제
public ResponseEntity<Article> deleteArticle(Long id){
if(bordrespository.existsById(id)){
bordrespository.deleteById(id);
return ResponseEntity.noContent().build();
}
else{
return ResponseEntity.notFound().build();
}
}
Comment on lines +54 to +61
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

existsById로 SELETE 쿼리 발생 후 deleteById로 DELETE 쿼리는 불필요한 2중 쿼리 같은데 차라리 try-catch문을 이용하여 DELETE 쿼리에서 예외 발생 시 이를 캐치하여 NotFound를 반환하게 하는게 더 좋아 보입니다

}
Comment on lines +1 to +62
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

하나의 클래스에 애플리케이션의 모든 비즈니스 로직을 포함하는 건 좋지 않습니다,각 로직별로 클래스를 분리해 주세요