Skip to content
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

#198 [Refactor] 모델 회원가입 API Service 분리 #217

Merged
merged 8 commits into from
Jan 31, 2024

Conversation

hellozo0
Copy link
Member

관련 이슈번호


## 해결하는 데 얼마나 걸렸나요? (예상 작업 시간 / 실제 작업 시간) * 2h / 3~4h

해결하려는 문제가 무엇인가요?

  • 모델 회원가입 API가 AuthController에 있었는데, ModelController로 옮기고
  • RegisterService로 모델 생성하는 부분 로직을 옮기는 작업을 하려고 했습니다!

고민이 되는 부분

  • ModelRegisterservice > createModel > if (modelJpaRepository.existsById(userId)) throw new ConflictException(ErrorCode.ALREADY_EXIST_MODEL_EXCEPTION); 해당 코드를 ModelRetrieveService로 처음에 옮겨서 isModelExist 라는 메소드를 만들었는데 분리하고자 했던 이유는 repository의존성 주입이 필요하지 않은 곳에서 모델이 존재하는지 확인하는 코드가 필요하지 않을까? 생각해서 분리하려고 하였으나 없는것 같아서 분리하지 않았음
  • Builder가 있었던 부분을 삭제하고 생성자를 만들었는데, 파트장님이 Builder 사용을 줄이면 좋다고 한 이유가 뭐였는지 기억이 안나서 왜 Builder 사용을 자제하라고 했지? 아티클을 찾아보면서 계속 생각을 해봤답니다..! 일단은 넘겨줄 값이 적어서 생성자 사용을 했습니다! --------------> Dto를 사용해서 넘겨 주라고 했는데 ModelRegisterservice > createModelPreferRegions > PreferRegion preferRegion = new PreferRegion(model, region); 이부분의 파라미터 값을 Dto로 넘겨주라는것이 파트장님의 이야기 일까요..? 고민을 하다가 이미 Dto로 request값을 받아서 고민하다 사용하지 않았답니다..

트러블 슈팅

순환참조 관련 문제 요약 & 느낀점 & 트러블 슈팅 기록


어떻게 해결했나요?

모델 생성 제대로 작동하는지 확인~! 완료
스크린샷 2024-01-31 오전 1 01 02
스크린샷 2024-01-31 오전 1 02 03
스크린샷 2024-01-31 오전 1 02 20

@hellozo0 hellozo0 requested review from KWY0218 and pkl0912 January 30, 2024 16:31
@hellozo0 hellozo0 self-assigned this Jan 30, 2024
Copy link
Member

@KWY0218 KWY0218 left a comment

Choose a reason for hiding this comment

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

작업하시느라 고생하셨습니다! 코맨트 확인해주세요!

왜 Builder 사용을 자제하라고 했지?

Entity에서 Builder 사용을 자제하라고 했었습니다!
Entity는 보호해야 할 대상인데 Builder를 사용하면 Entity의 모든 속성을 접근할 수 있기 때문에 Entity에서 Builder 사용을 지양하고 차라리 생성자를 사용하라고 했었습니다.
이번에 잘 적용하셨네요! 👍

파라미터 값을 Dto로 넘겨주라는것

서비스 계층의 Dto를 줘버리면 도메인에서 서비스의 의존성이 생기기 때문에 도메인에 서비스 계층의 Dto를 넘겨주는 것은 아닌 것 같습니다..!
만약 도메인에 Dto를 넘겨주고 싶다면, 도메인 계층의 Dto를 만들어서
서비스 계층에서 서비스 Dto -> 도메인 Dto 로 변환한 다음에 넘겨준다면
도메인 계층이 서비스 계층을 참조하는 일은 막을 수 있겠다고 생각합니다!


@Tag(name = "Auth Controller", description = "로그인 및 회원 가입 관련 API 입니다.")
Copy link
Member

Choose a reason for hiding this comment

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

p4
혹시 이런 식으로 메서드 별로 태그 걸면 스웨거 상으로 평소처럼 보이는 것일까요?

Copy link
Member Author

Choose a reason for hiding this comment

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

@KWY0218 넵! 예를들어 상위 Controller 단에 'AuthController'태그를 넣고, 하위 메소드 단에 추가로 'ModelController'를 달아두면 두 컨트롤러 단에 다 올라가더라고요 😓

Comment on lines +40 to +43
public PreferRegion(Model model, Region region) {
this.model = model;
this.region = region;
}
Copy link
Member

Choose a reason for hiding this comment

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

p3
👍
이제 빌더를 사용하지 않는다면 위에 @SuperBuilder 어노테이션도 삭제해주시면 좋을 것 같습니다!
만약 다른 곳에서 빌더 사용 중이면 다음에 없애면 될 것 같습니다!

@hellozo0 hellozo0 merged commit d085aa0 into develop Jan 31, 2024
1 check passed
@hellozo0 hellozo0 deleted the refactor/#198 branch January 31, 2024 06:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[refactor] 모델 회원가입 API 조회 컨트롤러/서비스 & 등록 컨트롤러/서비스 분리
2 participants