[ITDS-91] refactor: 공고 업로드 오류 수정 및 코드 품질 개선 #93
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
wongbingg
reviewed
Jun 15, 2025
Comment on lines
302
to
+311
| .map { $0.post } | ||
| .filter { !$0.title.isEmpty } | ||
| .filter { $0.hasContents } | ||
| .distinctUntilChanged() | ||
| .asSignal(onErrorSignalWith: .empty()) | ||
| .emit(to: postBinder) | ||
| .disposed(by: disposeBag) | ||
|
|
||
| reactor.state | ||
| .map { $0.post } | ||
| .filter { !$0.title.isEmpty } | ||
| .filter { $0.hasContents } |
Collaborator
There was a problem hiding this comment.
네이밍이 더 직관적으로 바뀌어서 좋네요 👍
wongbingg
reviewed
Jun 15, 2025
Comment on lines
+118
to
125
| public func mapDateValuesToResponseFormat() -> Post { | ||
| return updated( | ||
| recruitStartDate: translateToResponseFormat(for: recruitStartDate), | ||
| recruitEndDate: translateToResponseFormat(for: recruitEndDate), | ||
| activityStartDate: translateToResponseFormat(for: activityStartDate), | ||
| activityEndDate: translateToResponseFormat(for: activityEndDate) | ||
| ) | ||
| } |
Collaborator
There was a problem hiding this comment.
이렇게 특정 프로퍼티만 업데이트 하고싶은 경우 PropertyBuilder 를 사용해볼 수 있을 거 같아요
public func mapDateValuesToResponseFormat() -> Post {
return Post().builder
.recruitStartDate(translateToResponseFormat(for: recruitStartDate)
.recruitEndDate(translateToResponseFormat(for: recruitEndDate)
.activityStartDate(translateToResponseFormat(for: activityStartDate)
.activityEndDate(translateToResponseFormat(for: activityEndDate)
.build()
}추후 도입을 고려해봐도 괜찮을 거 같아요
Member
Author
There was a problem hiding this comment.
return self.builder... 이런 식으로 접근하면 기존 값을 유지한 채로도 특정 값을 업데이트 할 수 있어서 편리하겠군요!!👍
다만 구조체 외부에서 활용하기 위해선 프로퍼티의 setter 권한을 모두 열어둬야 해서 이번 경우엔 기존 방식을 유지하고, 추후에 비슷한 상황에선 적극적으로 고려해보도록 하겠습니다!
wongbingg
reviewed
Jun 15, 2025
Comment on lines
+73
to
+102
| public func updated( | ||
| imageData: Data? = nil, | ||
| title: String? = nil, | ||
| categoryTitle: String? = nil, | ||
| organization: String? = nil, | ||
| viewCount: Int? = nil, | ||
| recruitStartDate: String? = nil, | ||
| recruitEndDate: String? = nil, | ||
| jobGroups: [String]? = nil, | ||
| activityStartDate: String? = nil, | ||
| activityEndDate: String? = nil, | ||
| activityContents: String? = nil, | ||
| postUrlString: String? = nil, | ||
| isBookmarked: Bool? = nil | ||
| ) -> Post { | ||
| return Post( | ||
| imageData: imageData ?? self.imageData, | ||
| title: title ?? self.title, | ||
| categoryTitle: categoryTitle ?? self.categoryTitle, | ||
| organization: organization ?? self.organization, | ||
| viewCount: viewCount ?? self.viewCount, | ||
| recruitStartDate: recruitStartDate ?? self.recruitStartDate, | ||
| recruitEndDate: recruitEndDate ?? self.recruitEndDate, | ||
| jobGroups: jobGroups ?? self.jobGroups, | ||
| activityStartDate: activityStartDate ?? self.activityStartDate, | ||
| activityEndDate: activityEndDate ?? self.activityEndDate, | ||
| activityContents: activityContents ?? self.activityContents, | ||
| postUrlString: postUrlString ?? self.postUrlString, | ||
| isBookmarked: isBookmarked ?? self.isBookmarked | ||
| ) |
Collaborator
There was a problem hiding this comment.
입력되지 않은 다른 파라미터가 기본값 nil 로 적용되도 괜찮을까요? updated로 인해 반환되는 Post 객체 안의
다른 속성값이 유실되도 괜찮을지 확인이 필요해보여요
Member
Author
There was a problem hiding this comment.
함수 호출 시 변경이 필요한 값만 파라미터로 주입을 받기 위해 기본 값으로 nil을 지정했는데요,
반환하는 Post 객체의 생성부를 보시면 기본 값 nil이 들어오게 될 경우엔 언래핑을 통해 기존 값을 그대로 이어가게 해놨기 때문에 유실이 되진 않을 것 같습니다!
1. 파라미터 주입 -> 주입된 값으로 업데이트
2. 기본 값 nil -> 언래핑을 통해 기존 값 유지
wongbingg
reviewed
Jun 15, 2025
Comment on lines
48
to
+49
| switch action { | ||
| case .inputImage(let imageData): return .just(.setImage(imageData)) | ||
| case .inputTitle(let title): return .just(.setTitle(title)) | ||
| case .inputCategory(let category): return .just(.setCategory(category)) | ||
| case .inputOrganization(let organization): return .just(.setOrganization(organization)) | ||
| case .inputRecruitStartDate(let startDate): return .just(.setRecruitStartDate(startDate)) | ||
| case .inputRecruitEndDate(let endDate): return .just(.setRecruitEndDate(endDate)) | ||
| case .inputJobGroup(let jobGroups): return .just(.setJobGroup(jobGroups)) | ||
| case .inputActivityStartDate(let startDate): return .just(.setActivityStartDate(startDate)) | ||
| case .inputActivityEndDate(let endDate): return .just(.setActivityEndDate(endDate)) | ||
| case .inputActivityContents(let contents): return .just(.setActivityContents(contents)) | ||
| case .inputPostUrlString(let urlString): return .just(.setPostUrlString(urlString)) | ||
| case .updatePost(let newPost): return .just(.setPost(newPost)) |
wongbingg
approved these changes
Jun 15, 2025
Collaborator
wongbingg
left a comment
wongbingg
left a comment
There was a problem hiding this comment.
상진님 변경사항 모두 확인했습니다 ~!
간단한 코멘트 몇개 남겨두었습니다. 수정이 필요한 것은 없어서 추가작업 없으시면
바로 머지하셔도 될거같아요 고생하셨습니다 👍
Collaborator
저도 상황에 따라 유연하게 처리하는 것이 좋을 것 같아요! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
📌 관련 이슈 번호
📘 작업 유형
📙 작업 내역 (구현 내용 및 작업 내역을 기재합니다.)
📋 체크리스트 (PR을 올리기 전에 스스로 확인해봐요!)
📝 특이 사항
지난번에 원빈님께서 남겨주셨던 코멘트에 대해 고민해 보았는데,
데이터의 처리 로직이 복잡하거나 데이터의 양이 많은 경우엔 별도로 데이터 모델을 분리해 활용하는 것이 좋아보이고,
단순히 단일 데이터를 전달하는 경우엔 Observable 바인딩이나 Delegate 패턴을 활용하는 게 좋을 것 같다는 생각이 들었습니다!
굳이 하나의 방식으로 통일하기 보단 상황에 따라 유연하게 처리하는 게 좋을 것 같은데 혹시 원빈님은 어떻게 생각하시나요?