[한현승] Good-Night-3rd-Hackathon-Backend 제출 #21

82everywin · 2024-08-25T14:32:15Z

SpringBoot로 구현하였습니다.

주말동안 가족행사로 부득이하게 어제 저녁부터 시작하였습니다.
감사합니다.😊

TecheerTree/src/main/java/com/example/techeertree/controller/CommentController.java

jaeyeon93 · 2024-09-01T15:07:47Z

TecheerTree/src/main/java/com/example/techeertree/controller/CommentController.java

+    @Operation(summary = "댓글 등록")
+    @PostMapping("/{wishId}")
+    public ResponseEntity<CommentInfoResponseDto> createComment(@PathVariable Long wishId, @Valid @RequestBody CommentCreateRequestDto commentCreateRequestDto){
+        CommentInfoResponseDto responseDto = commentService.create(wishId, commentCreateRequestDto);


CommentInfoResponseDto는 Presentation Layer인데 비즈니스 레이어인 commentService에서 CommentInfoResponseDto를 반환하는거는 레이어 역행입니다.

이해했습니다! 감사합니다. 말씀해주신 토대로 Presentation Layer 와 Business Layer 에 맞춰서, 서비스, 컨트롤러 로직 수정 했습니다! 감사합니다!

jaeyeon93 · 2024-09-01T15:08:03Z

TecheerTree/src/main/java/com/example/techeertree/controller/CommentController.java

+    @Operation(summary = "댓글 조회")
+    @GetMapping("/{wishId}")
+    public ResponseEntity<Page<CommentInfoResponseDto>> getComments(@PathVariable Long wishId,


여기도 wishId들어가는게 이상합니다.

해당 소원에 댓글이 달려서 경로변수로 wishId를 넣고, 댓글 조회는 해당 소원의 모든 댓글을 확인할 수 있도록 하기 위해 wishId라고 적었는데 혹시, 변경한다면 id로 변경하는게 나은 방법일까요? 아니면 다른 방식이 더 나은가요??

wish는 별도의 리소스이기 때문에 WishController가 되어야될거 같아요.

jaeyeon93 · 2024-09-01T15:08:26Z

TecheerTree/src/main/java/com/example/techeertree/controller/CommentController.java

+                                                                   @RequestParam(defaultValue = "5") int size){
+


페이징 사이즈 5는 너무 작은데, 크게 잡으면 좋아요

처음 생각할 때, 소원이 많이 없을 것이라 생각하여, 페이징을 작게 잡은 것 같습니다! 10으로 잡겠습니다!

100으로 잡으면 괜찮습니다.

jaeyeon93 · 2024-09-01T15:09:12Z

TecheerTree/src/main/java/com/example/techeertree/controller/CommentController.java

+        Page<CommentInfoResponseDto> comments = commentService.getComments(wishId, page, size);
+        return ResponseEntity.status(HttpStatus.OK).body(comments);


여기도 위와 유사하게 레이어 역행인데, 서비스에서 List<CommentInfo> 같은걸 반환해서 여기서 ResponseDto로 매핑을 해줘야합니다.

감사합니다!
서비스 계층에서 Page<Comment> 값으로 반환하도록 하였고, 컨트롤러에서 Page<CommentInfoResponseDto>로 반환하도록 하여, dto를 컨트롤러에서 매핑을 했는데 이렇게 하는게 맞을까요?

네네 맞아요

jaeyeon93 · 2024-09-01T15:09:55Z

TecheerTree/src/main/java/com/example/techeertree/controller/WishController.java

+    public ResponseEntity<Void> deleteWish(@PathVariable Long id){
+        wishService.softDelete(id);
+        return ResponseEntity.noContent().build();
+    }


성공하면 HttpOk를 응답해줘도 될거같네요

상태 코드 204는 OK처리를 하지만, response body에 아무것도 담지 않을 때 사용한다고 배웠습니다. 그래서 삭제처리를 하면 204를 쓰는게 좋다고 배웠는데, soft-delete 처리를 할 때는 200처리가 더 나을까요?

그런 의견이면 204도 괜찮겠네요. 저는 그냥 200 OK를 사용해서 의견을 드려봤습니다.

jaeyeon93 · 2024-09-01T15:15:47Z

TecheerTree/src/main/java/com/example/techeertree/controller/WishController.java

+        List<Wish> pendingWishes = wishService.getPendingWishes();
+        List<WishInfoResponseDto> responseDtos= new ArrayList<>();
+        for(Wish wish : pendingWishes){
+            WishInfoResponseDto responseDto = WishInfoResponseDto.builder()
+                    .id(wish.getId())
+                    .title(wish.getTitle())
+                    .content(wish.getContent())
+                    .category(wish.getCategory())
+                    .build();
+            responseDtos.add(responseDto);


map을 활용하여 아래와 같이 작성하면 불필요한 변수나 리스트 사용을 안할거 같아요

List<WishInfoResponseDto> responseDtos = pendingWishes.stream().map(wish -> WishInfoResponseDto.ofCreate( wish.getId(), wish.getTitle(), wish.getContent(), wish.getCategory() )).toList(); public static class WishInfoResponseDto { private Long id; private String title; private String content; private Category category; private WishInfoResponseDto(Long id, String title, String content, Category category) { this.id = id; this.title = title; this.content = content; this.category = category; } public static WishInfoResponseDto ofCreate(Long id, String title, String content, Category category) { return new WishInfoResponseDto(id, title, content, category); } }

감사합니다!!

다만, 말씀 해주신 부분에서 WishInfoResponseDto에서 @Builder로 인해서 명시적인 생성자를 작성하지 않아도 오류없이 작동하기는 하는데, 이렇게 작성해도 될까요?

@Builder @Getter public static class WishInfoResponseDto { private Long id; private String title; private String content; private Category category; public static WishInfoResponseDto ofCreate(Long id, String title, String content, Category category) { return new WishInfoResponseDto(id, title, content, category); } }

�개인의 취향이라 상관없을거 같아요,

jaeyeon93 · 2024-09-01T15:18:59Z

TecheerTree/src/main/java/com/example/techeertree/dto/comment/CommentRequestDto.java

+    public static class CommentCreateRequestDto {
+
+        @NotBlank(message = "댓글은 필수 입력 항목입니다.")


취향의 차이이긴한데 저희는 static class를 사용하진 않고 다 클래스 분리합니다.

분리해서 사용한다면, 제가 작성한 static class가 모두 각 한 개씩 class로 분리하여 사용하신다는 말씀인거죠..?

jaeyeon93 · 2024-09-01T15:21:35Z

TecheerTree/src/main/java/com/example/techeertree/service/CommentService.java

+        Wish wish = wishRepository.findById(id)
+                        .filter(w -> {
+                            if (w.getIsConfirm() != Confirm.CONFIRM) {
+                                throw new BaseException(ErrorCode.NOT_CONFIRMED);
+                            }
+                            return true;
+                        })
+                        .orElseThrow(() -> new BaseException(ErrorCode.NOT_EXIST_WISH));


한번에 1가지 역할만하는게 좋을거 같은데, 우선 Wish를 찾고 나서 CONFRIRM 체크하는 비즈니스 로직을 if로 처리하면 가독성이 더 좋을거 같습니다.

말씀해주신 것처럼 filter로 계속 처리하기 보다는 if로 처리하는 게 훨씬 깔끔 하게 수정된 것 같습니다!

WishEntity wish = wishRepository.findById(id).orElseThrow(() -> new BaseException(ErrorCode.NOT_EXIST_WISH)); if(wish.getIsConfirm() != Confirm.CONFIRM) throw new BaseException(ErrorCode.NOT_EXIST_WISH);

이렇게 코드 수정을 해봤는데 괜찮을까요?

네네 상관없을거 같네요. wish.getIsConfirm() != Confirm.CONFIRM는 객체에서 isNotConfirmed()로 만들수 있겠네요

jaeyeon93 · 2024-09-01T15:22:54Z

TecheerTree/src/main/java/com/example/techeertree/repository/WishRepository.java

+import java.util.List;
+import java.util.Optional;
+
+public interface  WishRepository extends JpaRepository<Wish, Long> {


jpa엔티티뒤에는 postfix로 Entity를 붙이면 persistence layer라는걸 알 수 있어서 좋습니다. Wish가 순간 비즈니스 레이어인줄 알았어요

아 넵! 생각하지 못한 부분이라 미흡했던 것 같습니다! 수정하도록 하겠습니다!

jaeyeon93 · 2024-09-01T15:26:11Z

TecheerTree/src/main/java/com/example/techeertree/service/CommentService.java

+    @Transactional(readOnly = true)
+    public Page<CommentInfoResponseDto> getComments(Long wishId, int page, int size){


데이터 변경하는게 없기 때문에 굳이 @Transactional(readOnly=true)를 붙일필요는 없을거 같아요.
JPA 내부 read 메서드들에 @Transactional(readOnly=true 가 붙어있습니다.

감사합니다. @Transactional(readOnly=true)이 읽기 전용으로 쿼리를 진행하도록 하여 데이터의 무결성을 보장하기 위해 사용하였습니다. 그러나 이 어노테이션을 붙일 필요가 없다고 말씀 주신 이유가 데이터를 변경하는 것이 없기 때문에 굳이 필요하지 않다고 말씀 주신건지 궁금합니다.
또한 혹시, JPA 내부 메서드들에 @Transactional(readOnly=true)가 어노테이션 되어있기 때문에, 중복으로 할 필요가 없다는 의미로 말씀 주신 건지 궁금합니다.

둘 다 맞는데요. JPA 내부적으로 어노테이션이 붙어있기도 하고, 데이터 변경이 없기 때문에 @Transactional을 안붙여도 될거같다고 의견을 드렸습니다.

jaeyeon93 · 2024-09-01T15:26:33Z

TecheerTree/src/main/java/com/example/techeertree/service/CommentService.java

+        Wish wish = wishRepository.findById(wishId)
+                .filter(w -> {
+                    if(w.getIsConfirm() != Confirm.CONFIRM) {
+                        throw new BaseException(ErrorCode.NOT_CONFIRMED);
+                    }
+                    return true;
+                })
+                .filter(w -> {
+                    if(w.isDeleted()){
+                        throw new BaseException(ErrorCode.NOT_EXIST_WISH);
+                    }
+                    return true;
+                })
+                .orElseThrow(()-> new BaseException(ErrorCode.NOT_EXIST_WISH));


wish먼저 조회하고 filter치면 가독성이 더 좋을거 같아요

넵! 감사합니다! wish가 나온 후에 filter 로직을 적용하도록 하겠습니다!

jaeyeon93 · 2024-09-01T15:27:28Z

TecheerTree/src/main/java/com/example/techeertree/service/WishService.java

+
+@Service
+@RequiredArgsConstructor
+public class WishService {


비즈니스 레이어에서 resonseDto들을 반환하는건 적절하지 않습니다. 비즈니스 객체를 반환하고 Controller에서 객체 조립하는게 맞는거 같아요

그렇군요..! Service 계층에서는 비지니스로직을 처리하고, Wish 객체 자체를 반환하고, Controller에서 이 객체를 Dto에 맞게 조립했는데 제가 이해한 결과로 한 코드 수정이 맞을까요?

네네 맞아요

jaeyeon93 · 2024-09-01T15:28:31Z

TecheerTree/src/main/java/com/example/techeertree/service/WishService.java

+                })
+                .orElseThrow(()-> new BaseException(ErrorCode.NOT_EXIST_WISH));
+
+        return WishCreateMapper.INSTANCE.toDto(wish);


Mapper를 이렇게 사용하는 이유가 있나요??

persistence layer 층으로 구별가도록 수정했습니다.

82everywin added 21 commits August 24, 2024 18:18

환경설정

69f23f3

env: 의존성 추가

d0a27db

env: swagger 문서화

dd45d65

feat: wish 등록 기능 구현

acf52ee

feat: 예외처리 기능 구현

57dd26b

chore: gitignore 추가

29576ae

chore: gitignore

e48073e

feat: 소원 삭제 기능 구현

2b025d2

feat: 소원 승인/거절 처리 기능 구현 , refactor: Response 구현 수정

5684148

feat: 예외처리 추가 기능 구현

7a0e0ed

refactor: responsedto 구조체 수정

5e5f3a8

feat: 소원 단일 조회 기능 구현

f46f9b1

chore: 코드 정리

87f3166

feat: 소원 목록 조회 기능 구현

c35698a

chore: 코드 들여쓰기 수정

0865549

feat: 소원 등록 기능 구현

0f0cede

refactor: 트랜젝션 import 오류 수정

57700ee

feat: 댓글 등록 (승인된 소원에서만) 기능 구현

c70ea58

feat: 댓글 삭제 기능 구현

e8788b2

feat: 댓글 조회 기능 구현

1ec8e5a

feat: (추가기능) 소원 검색 기능 구현

c160c07

jaeyeon93 reviewed Sep 1, 2024

View reviewed changes

TecheerTree/src/main/java/com/example/techeertree/controller/CommentController.java Outdated Show resolved Hide resolved

jaeyeon93 reviewed Sep 1, 2024

View reviewed changes

82everywin added 13 commits September 11, 2024 13:45

refactor: 페이징 초기값 10으로 변경

3b2dd9c

refactor: @transactional 변경

72dde1e

refactor: Wish 엔티티 명칭 변경

38c49d1

persistence layer 층으로 구별가도록 수정했습니다.

refactor: 댓글등록 로직 수정

8f92088

refactor: Service계층 반환값 수정

69efd43

refactor: 비지니스로직 수정

da971b3

refactor: CommentController map 사용으로 변경

d43a3b5

refactor: 경로변수 변수명 변경

2e9c343

refactor : 레이어 역행 수정

b76df97

refactor: 페이지네이션 로직 수정 (레이어 역행)

f84c363

chore: import 삭제

ff1b416

gitignore

22726d3

refactor: Paging 로직 수정

ebe43bd

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[한현승] Good-Night-3rd-Hackathon-Backend 제출 #21

[한현승] Good-Night-3rd-Hackathon-Backend 제출 #21

82everywin commented Aug 25, 2024

jaeyeon93 Sep 1, 2024

82everywin Sep 12, 2024

jaeyeon93 Sep 1, 2024

82everywin Sep 11, 2024

jaeyeon93 Sep 16, 2024

jaeyeon93 Sep 1, 2024

82everywin Sep 11, 2024

jaeyeon93 Sep 16, 2024

jaeyeon93 Sep 1, 2024

82everywin Sep 12, 2024

jaeyeon93 Sep 16, 2024

jaeyeon93 Sep 1, 2024

82everywin Sep 11, 2024

jaeyeon93 Sep 16, 2024

jaeyeon93 Sep 1, 2024

82everywin Sep 11, 2024

jaeyeon93 Sep 16, 2024

jaeyeon93 Sep 1, 2024

82everywin Sep 11, 2024

jaeyeon93 Sep 1, 2024

82everywin Sep 11, 2024 •

edited

Loading

jaeyeon93 Sep 16, 2024

jaeyeon93 Sep 1, 2024

82everywin Sep 11, 2024

jaeyeon93 Sep 1, 2024

82everywin Sep 11, 2024 •

edited

Loading

jaeyeon93 Sep 16, 2024

jaeyeon93 Sep 1, 2024

82everywin Sep 11, 2024

jaeyeon93 Sep 1, 2024

82everywin Sep 11, 2024

jaeyeon93 Sep 16, 2024

jaeyeon93 Sep 1, 2024

		Page<CommentInfoResponseDto> comments = commentService.getComments(wishId, page, size);
		return ResponseEntity.status(HttpStatus.OK).body(comments);

		public static class CommentCreateRequestDto {

		@NotBlank(message = "댓글은 필수 입력 항목입니다.")

		@Transactional(readOnly = true)
		public Page<CommentInfoResponseDto> getComments(Long wishId, int page, int size){

[한현승] Good-Night-3rd-Hackathon-Backend 제출 #21

Are you sure you want to change the base?

[한현승] Good-Night-3rd-Hackathon-Backend 제출 #21

Conversation

82everywin commented Aug 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

82everywin Sep 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

82everywin Sep 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

82everywin Sep 11, 2024 •

edited

Loading

82everywin Sep 11, 2024 •

edited

Loading