[Spring Core] 박지빈 과제 제출합니다. by SCALAR127 · Pull Request #8 · BCSDLab-Edu/2024-BackEnd

SCALAR127 · 2024-06-03T13:39:53Z

No description provided.

Choi-JJunho

전체적으로 이해가 안가는 부분이 많아서 코멘트 남겼습니다.

대표적으로 controller 레벨에서 try - catch 를 사용한것이 이해가 잘 가지 않습니다.
AOP를 활용하는 이유를 잘 이해하지 못하고 있다는 생각이 드는군요

코멘트 참고하여 코드를 리팩터링하고 제안사항 반영해주시면 감사하겠습니다.

Choi-JJunho · 2024-06-04T13:38:26Z

src/main/java/com/example/demo/controller/ArticleController.java

+        }catch (DataIntegrityViolationException e){
+            final ErrorResponse response = ErrorResponse.of(ErrorCode.NOT_EXIST_USER);
+            return new ResponseEntity<>(response, HttpStatus.valueOf(response.getStatus()));
+        }


왜 RestControllerAdvice에서 안잡고 여기서 잡고있나요?

예를 들어, DataIntegrityViolationException 의 예외를 일으키는 다양한 원인이 있는데, 이것을 RestControllerAdvice에서 한번에 받으면, 모든 오류가 원인을 불문하고 한 가지의 ErrorCode로만 처리되게 되는데, 이를 해결할 수 없어서 controller에서 잡았습니다.

예외가 다소 추상적이라고 생각한다면 custom한 예외를 구현할 수 있을 것 같아요

Choi-JJunho · 2024-06-04T13:39:46Z

src/main/java/com/example/demo/controller/Error/ErrorCode.java

+    NOT_EXIST_ELEMENT(HttpStatus.NOT_FOUND, "존재하지 않는 요소입니다."),
+    EMAIL_DUPLICATION(HttpStatus.CONFLICT, "이미 존재하는 이메일입니다."),
+
+    NullPointerException(HttpStatus.BAD_REQUEST, "null인 값이 존재합니다. 다시 입력해주세요."),


이 값만 형식이 좀 이상한것 같은데 다른 값들은 UPPER_CASE로 작성하고 이 친구만 PascalCase로 작성한 이유가 있나요?

+) NullPointerException 은 언제 사용되는 에러코드인가요?
사용자가 이 문구를 보고 무엇이 null이길래 에러가 발생한건지 알 수 있을까요?

NullPointerException 에러를 잡고싶어서 NULL~로 시작하는 값을 만들고 싶었는데, Nu를 치니 NullPointerException을 자동완성할수 있어서 아무생각없이 tab키를 눌러 만든것 같습니다.

에러를 잡는것은 ControllerAdvice의 인자로 표현하면 됩니다~

Choi-JJunho · 2024-06-04T13:40:32Z

src/main/java/com/example/demo/controller/Error/ErrorExceptionHandler.java

+        return new ResponseEntity<>(response, HttpStatus.valueOf(response.getStatus()));
+    }
+
+    //@ExceptionHandler()


불필요한 주석은 제거해주세요

Choi-JJunho · 2024-06-04T13:41:35Z

src/main/java/com/example/demo/controller/Error/ErrorResponse.java

+        this.message = errorCode.getMessage();
+    }
+
+    public static ErrorResponse of(final ErrorCode errorCode){


Ctrl + Alt + L로 라인 포맷팅을 해달라는 피드백을 4번째.. 정도 드리고있는 것 같네요 ㅋㅋ

항상 라인 포맷팅하는 습관을 들이겠습니다.

Choi-JJunho · 2024-06-04T13:42:55Z

src/main/java/com/example/demo/repository/ArticleRepositoryJdbc.java

+                    INSERT INTO article (author_id, board_id, title, content)
                    VALUES (?, ?, ?, ?)
                    """,
                new String[]{"id"});
-            ps.setLong(1, article.getBoardId());
-            ps.setLong(2, article.getAuthorId());
+            ps.setLong(1, article.getAuthorId());
+            ps.setLong(2, article.getBoardId());
            ps.setString(3, article.getTitle());


굳이 바꿔준 이유가 있나요?

DTO만 바꾸면 되는데 잘 모르고 Domain도 바꿔버린것 같아요.

Choi-JJunho · 2024-06-04T13:45:18Z

src/main/java/com/example/demo/controller/dto/request/ArticleCreateRequest.java

@@ -1,10 +1,10 @@
 package com.example.demo.controller.dto.request;

 public record ArticleCreateRequest(


@JsonNaming 어노테이션을 활용하면 굳이 코드레벨에서 스네이크 케이스의 코드가 돌아다닐 필요가 없습니다.

Suggested change

public record ArticleCreateRequest(

@JsonNaming(PropertyNamingStrategy.SnakeCaseStrategy.class)

public record ArticleCreateRequest(

참고자료: https://www.baeldung.com/jackson-deserialize-snake-to-camel-case#use-jsonnaming-annotation

@JsonNaming이라는 어노테이션을 처음 보는데, 공부해서 코드에 실제 적용해 볼수 있도록 하겠습니다.

Choi-JJunho · 2024-06-04T13:46:16Z

src/main/java/com/example/demo/controller/dto/request/ArticleCreateRequest.java

+    Long board_id,
    String title,
-    String description
+    String content


변수명을 안바꾸고 구성할 수도 있습니다.

Suggested change

String content

@JsonProperty("content")

String description

참고자료: https://www.baeldung.com/jackson-deserialize-snake-to-camel-case#use-jsonproperty-annotation

확인했습니다

Choi-JJunho · 2024-06-04T13:47:19Z

src/main/java/com/example/demo/controller/Error/ErrorExceptionHandler.java

+public class ErrorExceptionHandler {
+
+    @ExceptionHandler(org.springframework.dao.EmptyResultDataAccessException.class)
+    public ResponseEntity<?> EmptyResultDataAccessException(EmptyResultDataAccessException e){


제네릭에 와일드카드를 선언한 이유가 있나요?

Suggested change

public ResponseEntity<?> EmptyResultDataAccessException(EmptyResultDataAccessException e){

public ResponseEntity<ErrorResponse> EmptyResultDataAccessException(EmptyResultDataAccessException e){

와일드카드가 지양되야 하는 것을 모르고 사용했습니다.

SCALAR127 · 2024-06-05T10:11:42Z

pr 반영해서 commit 했습니다! 하지만, 처음 try-catch를 대체할 수 있도록 RestControllerAdvice에서 코드를 구현하는 부분을 어떻게 바꿀수 있는지 고민해봐도 떠오르지 않아서 버디님이라면 어떤 방법으로 구현하실지 궁금합니다.

Choi-JJunho

잘 적용해주셨네요~
질문주신 내용에 대한 의견 달아봤습니다

Choi-JJunho · 2024-06-10T12:15:56Z

src/main/java/com/example/demo/controller/MemberController.java

+        try {
+            MemberResponse response = memberService.update(id, request);
+            return ResponseEntity.ok(response);
+        } catch (DuplicateKeyException e) {


pr 반영해서 commit 했습니다! 하지만, 처음 try-catch를 대체할 수 있도록 RestControllerAdvice에서 코드를 구현하는 부분을 어떻게 바꿀수 있는지 고민해봐도 떠오르지 않아서 버디님이라면 어떤 방법으로 구현하실지 궁금합니다.

Service 레이어에서 해당 예외를 catch한 뒤 내가 핸들링하고싶은 상황에 대한 새로운 예외를 만들어서 그 예외를 반환해볼 수 있을 것 같아요

try-catch는 Controller의 책임인가? 도 한번 생각해보시면 좋을 것 같아요 :)

보통 controller는 최대한 간단하고 역할이 없게 만들고, Service 에서 try-catch를 사용한다고 들었습니다.

SCALAR127 added 2 commits June 3, 2024 22:38

commit new

e428059

commit final

6f6895e

SCALAR127 changed the title ~~commit new~~ 박지빈 과제 제출합니다. Jun 3, 2024

SCALAR127 changed the title ~~박지빈 과제 제출합니다.~~ [Spring Core] 박지빈 과제 제출합니다. Jun 3, 2024

Choi-JJunho reviewed Jun 4, 2024

View reviewed changes

pr 반영 및 @Valid 어노테이션 활용

e4c4618

Choi-JJunho reviewed Jun 10, 2024

View reviewed changes

		@@ -1,10 +1,10 @@
		package com.example.demo.controller.dto.request;

		public record ArticleCreateRequest(

	public record ArticleCreateRequest(
	@JsonNaming(PropertyNamingStrategy.SnakeCaseStrategy.class)
	public record ArticleCreateRequest(

	public ResponseEntity<?> EmptyResultDataAccessException(EmptyResultDataAccessException e){
	public ResponseEntity<ErrorResponse> EmptyResultDataAccessException(EmptyResultDataAccessException e){

Conversation

SCALAR127 commented Jun 3, 2024

Uh oh!

Choi-JJunho left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SCALAR127 commented Jun 5, 2024

Uh oh!

Choi-JJunho left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants