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

[질문] 멘토님! 파일 패키지 구조 및 validation 관련해 질문 드립니다! #46

Open
park0503 opened this issue May 21, 2022 · 5 comments
Assignees

Comments

@park0503
Copy link
Member

park0503 commented May 21, 2022

현재 파일 패키지 구조 관련 리팩토링을 진행할 예정입니다.
지금은 controller, service, repository 등 layer 별로 패키지가 나누어져 있는데
이를 우선 travel, schedule 등의 domain으로 나눈 다음
그 안에서 service, repository 등으로 다시 나눌려고 합니다.
이 때 dto의 위치가 애매한데, 원래는 dto가 프론트엔드와의 통신을 위한 객체라는 점에서
controller 패키지 내에 dto 패키지를 따로 두었습니다.
이번에는 dto 패키지를 controller 밖으로 빼 service, repository 등과
나란히 배치 시키자는 의견이 나와서 현재 토의 중입니다.
이 안건 관련해서 멘토님의 의견을 듣고 싶습니다.

추가로 현재 database에 원하는 record가 존재하는지에 대한 확인을
따로 모아서 하지 않고 필요한 부분마다 orElseThrow를 이용해 예외를 처리해 주고 있습니다.
반복되는 코드가 많아 이를 메소드나 클래스로 추출하려고 하는데,
Validator 클래스를 만들어 해당 클래스에 모든 레포지토리를 주입시켜
record의 존재 여부에 대한 validation의 책임을 한 클래스에만 위임 시키자는 의견이 나왔습니다.
이 의견에 대해서도 멘토님의 의견을 듣고 싶습니다.

시간 되실 때 천천히 답변 남겨주시면 감사하겠습니다!

@park0503 park0503 assigned Sophoca and ErranderLee and unassigned Sophoca and ErranderLee May 21, 2022
@Sophoca
Copy link
Member

Sophoca commented May 22, 2022

@kmg8280

부연 설명을 보태자면 현재 파일 구조는 다음과 같습니다.

├── service
│   ├── TravelService
│   ├── ScheduleService
│   └── ...
├── controller
│   ├── travel
│   │   ├── dto
│   │   │   ├── TravelCreateReequestDto
│   │   │   └── ...
│   │   └── TravelController
│   └── ...

변경하고자 하는 파일 구조는 다음과 같습니다.

1안을 채택할 경우 service의 재사용성을 높이기 위해 controller에서 dto를 분리하여 service에 각 인자로 넣어주는 방식으로 추가적인 수정이 필요하지 않을까 생각됩니다.
2안의 경우는 현재 코드대로 controller에서 service를 호출할 때, 인자로 dto를 직접 넣어주는 방식을 사용할 것 같습니다. Dto의 전달 범위를 어디까지로 하느냐는 딱히 정해진 부분이 없다고 들어서 멘토님께 조언을 구하고자 합니다.

1안)
├── travel
│   ├── controller
│   │   ├── dto
│   │   │   ├── TravelCreateReequestDto
│   │   │   └── ...
│   │   └── TravelController
│   ├── domain
│   │   └── Travel
│   ├── repository
│   │   └── TravelRepository
│   └── service
│       └── TravelService
├── ...

2안)
├── travel
│   ├── controller
│   │   └── TravelController
│   ├── domain
│   │   └── Travel
│   ├── dto
│   │   ├── TravelCreateReequestDto
│   │   └── ...
│   ├── repository
│   │   └── TravelRepository
│   └── service
│       └── TravelService
├── ...

@kmg8280
Copy link

kmg8280 commented May 24, 2022

요즘 일이 바빠 확인이 늦어졌네요. 말씀주신 내용에선 2안이 좋아보입니다.
추가적인 내용은 오늘 밤늦게나 내일 중으로 답변드릴게요.

@kmg8280
Copy link

kmg8280 commented May 25, 2022

@Sophoca 안녕하세요 :)
윗 코멘트에서 말씀드린 것처럼 2안이 좋아보입니다.

"controller에서 service를 호출할 때, 인자로 dto를 직접 넣어주는 방식을 사용할 것 같습니다."
이 부분도 크게 문제는 없어보입니다.

DTO 뜻 자체가 Data Transfer Object 이기 때문에 DTO 내부적으로 로직을 가지지 않게 처리만 잘해주시면 될 것 같네요.
DAO 개념도 한번 찾아보시고 적용하는 것을 추천드려요.

@Sophoca
Copy link
Member

Sophoca commented May 25, 2022

@kmg8280
바쁘신 와중에 늦은 시간임에도 답변 주셔서 감사합니다!

조언해주신대로 2안 방식으로 리팩토링을 진행하도록 하겠습니다.
다만 entityCreateRequestDto의 경우 내부에 해당 entity를 생성하여 반환하는 toEntity() method를 가지고 있는 케이스가 몇 개 있는데 이를 제외하면 dto 내부에 별도의 로직을 가지지 않도록 처리해 둔 상태입니다. 혹시 이러한 부분도 service 단으로 분리하는게 좋을까요?
toEntity() 메소드가 있는 dto 중 하나인 postCreateRequestDto의 위치입니다.

추가적으로 앞서 @park0503 팀원이 이슈에 적은 두 번째 내용,
validation을 처리하기 위한 로직을 특정 클래스로 분리하는 리팩토링이 필요할까요?
대다수의 service에서 자주 사용하는 userRepository나 travelRepository의 경우
모든 service에서 findById() 등 DB 조회를 요청할 때, validation을 위해 exception handling을 하는 코드가 중복되는 상황입니다.
때문에 코드 중복을 최소화하기 위해 validation만을 위한 클래스를 생성하여
모든 service에서 이를 사용하면 좋을 것 같다는 생각이 들었습니다.

임시로 TravelService에서 해당 로직을 분리하여 만들어 둔 상태이며
추후 이를 클래스로 분리하여 다른 Service에서도 사용하는 것이 괜찮을까요?
해당 코드의 위치입니다.

늘 좋은 조언 주셔서 많은 도움 받고 있습니다 :)

@kmg8280
Copy link

kmg8280 commented May 28, 2022

@Sophoca

네, DTO에서 toEntity는 빼는 것이 좋아 보입니다.
entity 관련된 부분은 entity class에서 처리해주세요. 예전에 effective java 책 알려드린 적 있었는데 참고하시면 될 것 같아요.
https://devlog-wjdrbs96.tistory.com/256
entity class에서 정적 팩토리 메소드 만들어서 처리하시면 괜찮을 것 같습니다.

validation 같은 경우에는 각 entity class에서 관련 메소드 만들어서 처리하시고 관련 exception정도만 throw 해서 error handling은 GlobalExceptionHandler에서 처리해도 괜찮을 것 같아요. 이 부분은 편하신대로 구현해주세요 ㅎㅎ;

Sevice class에서 validation 메소드를 전부 구현해놓으면 Service class 자체가 너무 커질 것 같아요.

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

No branches or pull requests

4 participants