[Network Refactoring] Day 열거형 타입을 Codable 처리 #276
Merged
Conversation
This file contains 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
Day라는 열거형이 존재하지만, Codable을 준수하는 모델에서는 `Day`를 받지않고 `String`타입을 받고 있었습니다. 그렇게 String타입을 받고 다시 Day로 파싱하는 과정이 불필요한 과정이라 생각하여 API를 파싱하여 받을 때 Day로 받도록 처리했습니다. 이 과정에서 String extension을 제거할 수 있었고, day가 갖고있는 compute property도 깔끔하게 정리할 수 있었습니다.
WhiteHyun
added
✋ Help wanted
soobin-k
requested changes
Apr 10, 2023
| @@ -31,7 +31,7 @@ struct DetailRecruitmentResponse: Codable { | |||
| let mainImage: String? | |||
|
|
|||
| /// 플러빙 모임 요일 | |||
| let days: [String] | |||
| let days: [Day] | |||
There was a problem hiding this comment.
승현님 이거 서버에서 한글로 내려줬던 것 같아서 이렇게 파싱하면 안될 것 같습니다!!
모임 생성할때는 영어로 보내고 (["MON", "TUE"])
모집글 상세 조회 시엔 한글로 내려왔던 것 같아요!
"days": [
"목",
"금",
"토",
"일"
],
There was a problem hiding this comment.
헉 확인했습니다..! 모집 상세 조회에서만 그러는 거군요..
이 녀석 하나로 인해 String으로 바꾸기에는 리스크(?)가 커서, Decoder로 Day로 받도록 처리할 수는 있을 것 같아요.
이 쪽은 제가 한 번 알아보고 건드려보겠습니다..!
고마워요 수빈님
There was a problem hiding this comment.
Decoder를 이용해서 String으로 파싱 한 뒤 Day로 초기화하는 작업을 거쳐 정상적으로 값이 들어오도록 처리했습니다.
init(from decoder: Decoder) throws {
let container = try decoder.container(keyedBy: CodingKeys.self)
// codes ...
days = try container.decode([String].self, forKey: .days).compactMap { Day(rawValue: $0) }
// codes ...
}모집 상세 조회 시 요일이 잘 들어오는 것을 확인할 수 있었습니다!
요일이 한글로 들어오면서 Day가 파싱되지 않는 버그가 발생했습니다. 해결하기 위해 decoder를 이용하여 String으로 파싱한 뒤 Day로 초기화하는 작업을 거쳤습니다.
soobin-k
approved these changes
Apr 11, 2023
| name = try container.decode(String.self, forKey: .name) | ||
| goal = try container.decode(String.self, forKey: .goal) | ||
| mainImage = try container.decodeIfPresent(String.self, forKey: .mainImage) | ||
| days = try container.decode([String].self, forKey: .days).compactMap { Day(rawValue: $0) } |
Comment on lines
+20
to
+32
| init?(rawValue: String) { | ||
| switch rawValue { | ||
| case "월", "MON": self = .monday | ||
| case "화", "TUE": self = .tuesday | ||
| case "수", "WED": self = .wednesday | ||
| case "목", "THR": self = .thursday | ||
| case "금", "FRI": self = .friday | ||
| case "토", "SAT": self = .saturday | ||
| case "일", "SUN": self = .sunday | ||
| case "전체", "요일 무관", "ALL": self = .all | ||
| default: return nil | ||
| } | ||
| } |
Merged
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 요약
🌱 작업한 내용
Day라는 타입을Codable하도록 만들었고, 요일을 받거나 요청하는 모델에String타입 대신Day타입을 받도록 처리했습니다.이렇게 함으로써 String에서 Day로 파싱하는 단계를 없앨 수 있었고, 파싱하는 모델인
Ex+String도 삭제했습니다.Day에서 한글과 영어로 바꾸기 위한
Compute Property에서eng를 담당하는 값을rawValue가 갖도록 처리했습니다.정리
리팩토링 하면서 얻은 이점
단점
필요한 리팩토링이라 생각하여 구현하였으나, 전달드리지 않은 상태에서 여러분의 로직을 일부 수정하게 되어 죄송한 말씀을 드립니다.
해당 코드를 확인하시고, 잘못된 점이 있거나 하면 언제든 리뷰 넣어주세요. 제가 테스트했을 때는 전부 정상적으로 동작했으나, 제가 놓친 부분도 있을 수 있기에 여러분의 관심이 필요합니다 :)
📮 관련 이슈