Skip to content

REAFACTOR: Domain Layer DTO 제거 #120

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.

Sign up for GitHub

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

Jump to bottom
Merged
zzangzzangguy merged 12 commits into from
Apr 20, 2025
Merged

REAFACTOR: Domain Layer DTO 제거 #120

zzangzzangguy merged 12 commits into from
Apr 20, 2025

Conversation

@zzangzzangguy
Copy link
Contributor

@zzangzzangguy zzangzzangguy commented Apr 20, 2025 •
edited by 0Hooni
Loading

📌 이슈

✅ 작업 사항

  • MapAPI 관련 DTO 제거 UI(NMapsMap, toStoreItem 등) 의존성 제거
  • Auth API 관련 DTO 제거
  • Comment API 관련 DTO 제거
  • Admin API 관련 DTO 제거 별도 Params 추가

@zzangzzangguy zzangzzangguy added the 🔄 refactor 프로덕션 코드 리팩토링, 파일 삭제, 네이밍 수정 및 폴더링 label Apr 20, 2025
@zzangzzangguy zzangzzangguy self-assigned this Apr 20, 2025
@zzangzzangguy zzangzzangguy linked an issue Apr 20, 2025 that may be closed by this pull request
Domain Layer에서 DTO 제거 (Admin, Auth, Comment, Map, MapDirection) #119
Closed
2 tasks
@0Hooni 0Hooni reopened this Apr 20, 2025
@0Hooni 0Hooni changed the base branch from develop to refactor/#112-Modulization April 20, 2025 01:08
0Hooni
0Hooni approved these changes Apr 20, 2025
View reviewed changes
Copy link
Member

@0Hooni 0Hooni left a comment

Choose a reason for hiding this comment

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

수고하셨습니다!

DTO랑 Entity 분리가 필요한 작업이 하나 있는것 같은데 확인 후 수정 부탁드립니다

dongglehada
dongglehada suggested changes Apr 20, 2025
View reviewed changes
Copy link
Member

@dongglehada dongglehada left a comment

Choose a reason for hiding this comment

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

DTO에서 Entity로 변환할 때 통일된 형태로 변환을 진행하는 것 같지 않습니다. 프로젝트에서 일반적으로 변환하는 과정에 맞춰 통일하는 것이 좋아보이며 Entity의 네이밍 및 파일 분리가 필요할 것 같습니다.

Poppool/Poppool/DomainLayer/DomainInterface/Entity/AdminResponse/Params/AdminParams.swift
Copy link
Member

@dongglehada dongglehada Apr 20, 2025

Choose a reason for hiding this comment

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

Entity가 프로젝트에서 일반적으로 사용되고 있던 네이밍이 아니라 조금 헷갈리지 않을까 하는 생각이 듭니다..!

Poppool/Poppool/DataLayer/RepositoryImpl/Converter/MapDomainModelConverter.swift Outdated
Copy link
Member

@dongglehada dongglehada Apr 20, 2025

Choose a reason for hiding this comment

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

MapDomainModelConverter가 필요한 이유가 있을까요? MapPopUpStoreDTO에 메서드를 추가하는 방식 말고 왜 이방식으로 Entity로 변환하는지 궁금합니다.

Copy link
Contributor Author

@zzangzzangguy zzangzzangguy Apr 20, 2025

Choose a reason for hiding this comment

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

대부분의 DTO에서 toDoMain으로 매핑하여 처리하는것에 대한 의문에서 시작했습니다 .. 이부분만 우선적으로 진행해뒀습니다만
네트워크레이어에서 매핑까지 하는것보다 도메인 모델이 DTO에 조금의 의존성도 가지지 않으려면 분리하는게 좋지않을까 ? 란생각으로 분리하게되었습니다 DTO 마다 ToDoMain을 제각각 구현하는것보다 각기 컨버터 를 추가하는것도 나쁘지않을것 같아서요!

Poppool/Poppool/DataLayer/Network/API/MapAPI/ResponseDTO/MapPopUpStoreDTO.swift
Copy link
Member

@dongglehada dongglehada Apr 20, 2025

Choose a reason for hiding this comment

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

해당 부분이 삭제되고 컨버터가 생긴것일까요? 프로젝트의 일반적인 구조와 다르게 사용한 이유가 궁금합니다

Poppool/Poppool/DataLayer/RepositoryImpl/AdminRepositoryImpl.swift
import Foundation

import Alamofire
import Foundation
Copy link
Member

@dongglehada dongglehada Apr 20, 2025

Choose a reason for hiding this comment

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

임포트 순서가 퍼스트 파티, 서드 파티 순서로 배치되어 있으면 좋을 것 같습니다.

@zzangzzangguy
Merge branch 'refactor/#112-Modulization' of https://github.com/PopPo…
ab5403d 
…ol/iOS-renew into refactor/#117-remove-dtos-domain-layer

# Conflicts:
#	Poppool/Poppool/PresentationLayer/Presentation/Scene/Admin/AdminReactor.swift
#	Poppool/Poppool/PresentationLayer/Presentation/Scene/Admin/AdminViewController.swift
#	Poppool/Poppool/PresentationLayer/Presentation/Scene/MyPage/Main/MyPageReactor.swift
@zzangzzangguy
Merge branch 'refactor/#117-remove-dtos-domain-layer' of https://gith…
7c88129 
…ub.com/PopPool/iOS-renew into refactor/#117-remove-dtos-domain-layer

# Conflicts:
#	Poppool/Poppool/PresentationLayer/Presentation/Scene/Admin/AdminReactor.swift
#	Poppool/Poppool/PresentationLayer/Presentation/Scene/Admin/AdminViewController.swift
#	Poppool/Poppool/PresentationLayer/Presentation/Scene/MyPage/Main/MyPageReactor.swift
dongglehada
dongglehada suggested changes Apr 20, 2025
View reviewed changes
Copy link
Member

@dongglehada dongglehada left a comment

Choose a reason for hiding this comment

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

DTO 사용부가 확인됩니다. 해당 부분 수정 부탁드릴게요!!

Poppool/Poppool/DataLayer/Network/API/AdminAPI/AdminAPIEndpoint.swift
Copy link
Member

@dongglehada dongglehada Apr 20, 2025

Choose a reason for hiding this comment

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

Completable 사용이 좋을 것 같습니다

Poppool/Poppool/PresentationLayer/Presentation/Scene/Admin/AdminViewController.swift Outdated
)) { _, item, cell in
cell.configure(with: item)
)) { _, store, cell in
let dto = GetAdminPopUpStoreListResponseDTO.PopUpStore(
Copy link
Member

@dongglehada dongglehada Apr 20, 2025

Choose a reason for hiding this comment

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

DTO 사용이 보입니다.

dongglehada
dongglehada approved these changes Apr 20, 2025
View reviewed changes
Copy link
Member

@dongglehada dongglehada left a comment

Choose a reason for hiding this comment

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

LGTM!! 확인 완료하였습니다!

0Hooni
0Hooni approved these changes Apr 20, 2025
View reviewed changes
Copy link
Member

@0Hooni 0Hooni left a comment

Choose a reason for hiding this comment

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

LGTM!!

@zzangzzangguy zzangzzangguy merged commit 9ed3c9e into refactor/#112-Modulization Apr 20, 2025
@0Hooni 0Hooni deleted the refactor/#117-remove-dtos-domain-layer branch October 19, 2025 11:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@0Hooni 0Hooni 0Hooni approved these changes

+1 more reviewer

@dongglehada dongglehada dongglehada approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@zzangzzangguy zzangzzangguy

Labels

🔄 refactor 프로덕션 코드 리팩토링, 파일 삭제, 네이밍 수정 및 폴더링

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Domain Layer에서 DTO 제거 (Admin, Auth, Comment, Map, MapDirection)

4 participants

@zzangzzangguy @0Hooni @dongglehada