#11 Answer 패키지 리팩토링 #15

LeeJaeYun7 · 2023-08-15T13:08:36Z

Answer 패키지를 리팩토링했습니다

f-lab-moony

고생 많으셨어요~ 코멘트를 좀 남겨 두었는데, 확인 후 리뷰 재요청 부탁드려요 ~

f-lab-moony · 2023-08-17T23:57:10Z

src/main/java/com/example/interviewPrep/quiz/answer/controller/AnswerController.java

+    }
+
+    @PostMapping("/api/v1/answer")
+    public ResultResponse<?> createAnswer(@RequestBody @Valid AnswerRequest answerRequest){


@Valid는 어떤 역할을 하고 있나요 ?

지금은 AnswerRequest에 Objects.requireNonNull을 추가해서 따로 필요하지 않을 것 같습니다.

@Valid는 DTO 필드들에 @NotNull, @Email 등이 쓰인다면, 해당 필드에 대해 검증해주는 역할을 맡습니다.

https://jyami.tistory.com/55

반영 완료

f-lab-moony · 2023-08-17T23:58:17Z

src/main/java/com/example/interviewPrep/quiz/answer/controller/AnswerController.java



-    @GetMapping("/{id}")
+    @GetMapping("/api/v1/answer/{id}")


엔드포인트는 메서드 별로 관리를 하기로 하신걸까요 ~? prefix는 컨트롤러를 나누는 기준이 되기도 해서 클래스 레벨에 붙여놔도 좋을 것 같아요 ~

넵 확인했습니다

반영 완료

f-lab-moony · 2023-08-18T00:00:38Z

src/main/java/com/example/interviewPrep/quiz/answer/domain/Answer.java


-    public int commentIncrease() { return ++this.commentCnt; }
+    public int commentIncrease() {
+        return ++this.commentCnt;


동시에 동일한 답변에 코멘트가 달리는 경우, 이 메서드는 스레드세이프하게 동작할 수 있을까요 ?

스레드 세이프 하게 동작하지 않을 것 같습니다.
스레드 세이프 하게 동작하도록 수정해야 할 것 같습니다

반영 완료

f-lab-moony · 2023-08-18T00:01:07Z

src/main/java/com/example/interviewPrep/quiz/answer/dto/request/AnswerRequest.java

+import java.util.Objects;
+
+@Getter
+@Builder


빌더를 생성자에 붙이는걸 습관화하면 좋을 것 같아요 ~

넵 확인했습니다

반영 완료

f-lab-moony · 2023-08-18T00:08:37Z

src/main/java/com/example/interviewPrep/quiz/answer/repository/AnswerRepository.java


    @Query("select a.question.id from Answer a where a.question.id in ?1 and a.member.id = ?2 ")
-    List<Long> findMyAnswer(List<Long> qList, Long memberId); //@Param("memberId")
+    List<Long> findMyAnswer(List<Long> questionIds, Long memberId); //@Param("memberId")


f-lab-moony · 2023-08-18T00:09:44Z

src/main/java/com/example/interviewPrep/quiz/answer/service/AnswerService.java

-                .createDate(customLocalDateTime(answer.getCreatedDate()))
-                .name(answer.getMember().getName())
-                .build();
+        return AnswerResponse.builder()


요런건 정적 팩토리 메서드를 이용해보면 좋을 것 같아요 ~

넵 확인했습니다

반영 완료

f-lab-moony · 2023-08-18T00:11:33Z

src/main/java/com/example/interviewPrep/quiz/answer/service/AnswerService.java

        Long memberId = JwtUtil.getMemberId();
        Page<Answer> answers;

+        if(!(type.equals("my") || type.equals("others"))){


타입은 enum을 이용해보면 어떨까요 ?

넵 확인했습니다

반영 완료

f-lab-moony · 2023-08-18T00:12:57Z

src/main/java/com/example/interviewPrep/quiz/answer/service/AnswerService.java

        Long memberId = JwtUtil.getMemberId();
-        if(memberId==0L) throw new CommonException(NOT_FOUND_ID);
+
+        if(memberId==0L) {


memberId 0은 언제 들어가는건가요 ?

멤버가 로그인되어 있지 않은 상황에서는 JwtUtil.getMemberId()가 0L을 반환하도록 설정되어 있습니다

JwtUtil.getMemberId() 자체가 로그인 된 멤버의 아이디를 가져오겠다는 의미로 보이는데, 로그인되어있지 않은 경우 0을 반환 할 이유가 있을까요 ? 애초에 로그인되지 않은 경우 예외를 던지는게 어떨까 해서 의견 남겨봅니다 ~

넵 확인했습니다

반영 완료

f-lab-moony

고생 많으셨어요 ~
피드백 남겨 두었는데, 요건 머지 해 둘게요 ~

f-lab-moony · 2023-08-24T15:19:39Z

src/main/java/com/example/interviewPrep/quiz/answer/controller/AnswerController.java

+    @PostMapping()
+    public ResponseEntity<Void> createAnswer(@RequestBody AnswerRequest answerRequest){
+        answerService.createAnswer(answerRequest);
+        return ResponseEntity.status(HttpStatus.CREATED).build();


개인적으로 성공응답은 그냥 다 200으로 내려도 된다고 생각해요. 성공 상태에 따라서 뭐 분기할 일이 없어서 ..

f-lab-moony · 2023-08-24T15:21:42Z

src/main/java/com/example/interviewPrep/quiz/answer/domain/Answer.java

        this.version = version;
    }

+    public static Answer createAnswerEntity(Member member, Question question, String content){


이건 왜있는걸까요 ?

f-lab-moony · 2023-08-24T15:23:13Z

src/main/java/com/example/interviewPrep/quiz/answer/domain/Answer.java

    }

-    public int increase() {
+    public synchronized int increase() {


synchronized가 이런식으로 걸리면 성능에 어떤 영향이 있을까요 ? AtomicInteger에 대해서 찾아보시면 좋을 것 같아요 ~

f-lab-moony · 2023-08-24T15:29:00Z

src/main/java/com/example/interviewPrep/quiz/answer/dto/request/AnswerRequest.java


    private String content;

+    @Builder


요거 저번에 유효성검사 위치를 잘못 봤는데, 이 생성자에 유효성검사가 들어있으면 실제 요청시에 유효성검사 되나요 ?

추가로, 유효성검사가 요청 객체에 있는게 맞을지, 서비스 메서드에 있는게 맞을지 고민도 한번 해보시면 좋을 것 같아요 ~

f-lab-moony · 2023-08-24T15:30:56Z

src/main/java/com/example/interviewPrep/quiz/answer/service/AnswerService.java

        Page<Answer> answers;

-        if(!(type.equals("my") || type.equals("others"))){
+        Type inputType = Type.valueOf(type.toUpperCase());


컨트롤러에서 처음부터 Type으로 받으면 치환 안해줘도 되지 않을까요 ~?

f-lab-moony · 2023-08-24T15:31:55Z

src/main/java/com/example/interviewPrep/quiz/answer/service/AnswerService.java

-        if(!(type.equals("my") || type.equals("others"))){
+        Type inputType = Type.valueOf(type.toUpperCase());
+
+        if(!(inputType.equals(Type.MY) || inputType.equals(Type.OTHERS))){


Type에 포함되지 않은 녀석들을 거르려고 하신 거 같아요 ~ 위와 마찬가지로 컨트롤러에서 enum 타입으로 받으면 이것도 없어도 될 것 같습니다 ~

f-lab-moony · 2023-08-24T15:32:52Z

src/main/java/com/example/interviewPrep/quiz/utils/Type.java

@@ -0,0 +1,6 @@
+package com.example.interviewPrep.quiz.utils;
+
+public enum Type {


Type은 너무 범용적인 이름인데 좀 더 명확한 이름 없을까요 ?

#15 Question 패키지 버그 수정

#15 findByCompany 메소드 추가 with QueryDSL

Answer 패키지 리팩토링

e0b48a4

f-lab-moony requested changes Aug 18, 2023

View reviewed changes

LeeJaeYun7 added 2 commits August 21, 2023 17:12

PR 피드백 반영

6c78549

PR 피드백 반영2

7853b48

f-lab-moony approved these changes Aug 24, 2023

View reviewed changes

Merge branch 'main' into fix/AnswerRefactoring

da3a69d

f-lab-moony merged commit b18d888 into main Aug 24, 2023

LeeJaeYun7 added a commit that referenced this pull request Aug 28, 2023

Merge pull request #20 from f-lab-edu/fix/question

926b471

#15 Question 패키지 버그 수정

LeeJaeYun7 added a commit that referenced this pull request Sep 4, 2023

Merge pull request #21 from f-lab-edu/feat/questionByCompany

a1e3d61

#15 findByCompany 메소드 추가 with QueryDSL

		@@ -0,0 +1,6 @@
		package com.example.interviewPrep.quiz.utils;

		public enum Type {

#11 Answer 패키지 리팩토링 #15

#11 Answer 패키지 리팩토링 #15

Uh oh!

Conversation

LeeJaeYun7 commented Aug 15, 2023

Uh oh!

f-lab-moony 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!

LeeJaeYun7 Aug 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

f-lab-moony 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!

Reviewers

Assignees

Labels

Projects

LeeJaeYun7 Aug 21, 2023 •

edited

Loading