Skip to content

Conversation

@leebs0521
Copy link
Collaborator

@leebs0521 leebs0521 commented Nov 20, 2024

resolved :

📌 과제 설명

  • 봉사 활동 모집글 생성 기능 작성
    • 봉사 활동 모집글 저장
    • 봉사 활동 등록 정보에 존재하는 위치 엔티티 저장

👩‍💻 요구 사항과 구현 내용

✅ PR 포인트 & 궁금한 점

  • 서비스 마다 인터페이스 생성 후 구현체를 만들어서 작업했습니다.
  • 파사드 도입(UseCase)
    • 아무리 생각해도 파사드가 필요해보여서 추가했습니다. 읽고 코멘트 부탁드립니다.
    • 파사드 도입하면서 dto도 세분화 했습니다. 확인해보시고 필요성을 못느끼신다면 피드백 부탁드립니다.

@leebs0521 leebs0521 self-assigned this Nov 20, 2024
@leebs0521 leebs0521 linked an issue Nov 20, 2024 that may be closed by this pull request
2 tasks
@leebs0521 leebs0521 changed the title Feature/3 create recurit board [FEATURE] 봉사활동 모집글 생성 기능 Nov 20, 2024
@leebs0521 leebs0521 force-pushed the feature/3-create-recurit-board branch from 6cc207f to 2c19009 Compare November 20, 2024 03:32
@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 60%)

See analysis details on SonarQube Cloud

Copy link
Collaborator

@m-a-king m-a-king left a comment

Choose a reason for hiding this comment

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

범수 님, 수고하셨습니다!

코드에는 남기지 않았지만 몇 가지 질문이 있습니다.

  1. Service라는 접미사가 인터페이스 명명에 사용된 것
  2. 이미지를 Optional로 표현한 것

두 가지에 대해서 얘기를 나눠보고 싶습니다.


public interface LocationCommandService {

public Long createLocation(LocationCreateRequestDto dto);
Copy link
Collaborator

@m-a-king m-a-king Nov 20, 2024

Choose a reason for hiding this comment

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

인터페이스 메서드는 기본적으로 public으로 간주되는데, 이를 명시적으로 작성하는 것에 대해서 여쭤보고 싶습니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

아 이 부분은 실수입니다.. public 제거하는 방향을 수정하겠습니다 ㅎㅎ

@@ -0,0 +1,34 @@
package com.somemore.recruitboard.dto.command;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
package com.somemore.recruitboard.dto.command;
package com.somemore.recruitboard.dto.request;

저희 컨벤션이 위와 같지 않나요?
비슷한 이유로 레코드명에 Command가 들어가는 것에 대해서도 여쭤보고 싶습니다.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

음.. 컨벤션상으로 request가 맞긴합니다.
다만 해당 dto는 RecruitBoardUseCase에서 recruitBoardCommandService의 create() 메서드의 인자로 들어가는 dto 이기 때문에 명명을 다르게 하는게 맞다고 생각해서 작성했습니다.

제 나름대로의 규칙을 정한건.. dto.request 패키지는 컨트롤러에서 사용되는 dto, dto.command 패키지는 파사드 서비스에서 다른 서비스를 호출할 때 dto 정보가 달라질 경우 필요 정보만 담는 dto가 모인 패키지로 생각하고 넣긴했는데, 이부분 의논해보면 좋을 것 같습니다.

레코드명에 Command가 들어가는 이유도 비슷한 dto 패키지를 나눈 맥락과 동일하다고 말씀드릴수 있을 것 같아요.

Copy link
Collaborator

@ayoung-dev ayoung-dev left a comment

Choose a reason for hiding this comment

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

아직 제가 부족해서 보면서 제가 헷갈렸던 부분 도움이 됐어요
범수님 코드에 맞춰서 수정하여 저도 올리겠습니다!

파사드에 대해서는 저는 좋은 거 같아요!
수고하셨습니다~

아 PR 메시지에 브런치 내려서 앞에 - 붙이면 더 보기 좋을 거 같아요!

@NoArgsConstructor(access = PROTECTED)
@Entity
@Table(name = "Recruit_board")
public class RecruitBoard {
Copy link
Collaborator

Choose a reason for hiding this comment

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

BaseEntity 상속 일부러 아직 안 하신 건가요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

헉 죄송합니다.. 까먹었습니다..

private Long locationId;

@Column(name = "center_id", nullable = false)
private Long centerId;
Copy link
Collaborator

Choose a reason for hiding this comment

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

centerId는 UUID인 거 같아요

@leebs0521
Copy link
Collaborator Author

아까 회의 했던 대로 코드 변경하기엔 커밋 기록이 더러워질것 같아서 다시 작성하겠습니다.

@leebs0521 leebs0521 closed this Nov 20, 2024
@leebs0521 leebs0521 deleted the feature/3-create-recurit-board branch November 20, 2024 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants