오픈 마켓 [STEP 1] 숲재, yeha #90

hayeonhee · 2022-01-08T13:05:40Z

안녕하세요. 쿠마! @leejun6694
저희 드디어 Step1 PR 요청을 드리게 되었습니다.
요청에 필요한 메서드를 구현하는 것에 많은 시간과 공을 들였습니다...🧐
어떤 작은 조언도 큰 도움이 될 것같습니다. 잘 부탁드립니다!
감사합니다. 🙇

고민했던 점

URLSession을 활용한 네트워킹 모델 구현

APIService 프로토콜을 생성하고, 네트워킹에 필요한 메서드들을 extension부에 기본 구현했습니다.
- doDataTask메서드는 Generic메서드로 구현되었습니다.
- URLRequest 를 파라미터로 받아 dataTask를 수행하며 파싱을 진행하고 Result<T, Error> -> Void 형태의 CompletionHandler로 처리할 수 있게 설계했습니다.
ProductService는 상품 관련 API통신을 담당하는 타입입니다.
- APIService프로토콜을 채택하여 기본 구현된 메서드들을 사용해 RestfulAPI에 요청, 응답 및 파싱을 수행합니다.

UserDefaults의 사용

판매자 identifier 및 password를 UserDefault를 활용하여 UserDefaultUtility 타입에서 관리할 수 있도록 구현해 주었습니다.

해결이 되지 않은 점

개별 상품의 `secret`조회 시 디코딩 실패

문제 상황
- doDatatask 메서드 내에 JSON 데이터를 파싱하는 부분까지 모두 함께 처리하도록 했습니다.
- 개별 상품의 secret을 조회할 때, 응답으로 오는 secret 이 4df1f4fd-7011-11ec-abfa-7729db260f07 와 같은 따옴표가 없는 형태로 들어와서 디코딩이 되지 않습니다. (추측컨데, 따옴표가 없어 JSON으로 인식하지 못하는 듯 합니다.)
- JSONDecoder.decode 가 호출되기 전까지 기대한 값이 들어 있는 것을 확인했습니다. 그래서 디코딩에 문제가 있는 것을 알았습니다.
시도한 방법
- 아래와 같은 방식으로 테스터는 통과해 볼 수 있었습니다.
  - secret 의 앞뒤에 " " 를 붙여서 String 타입으로 만들어 decode메서드를 거치지 않고 바로 반환하도록 했습니다.
  - JSONDecoder.decode 메서드를 거칠 경우 String 타입으로 변환한 secret 을 다시 Data 타입으로 변환하고 디코딩을 해야 해서 거치지 않도록 바로 반환하는 방식으로 테스트를 진행했습니다.
궁금한 점
- 4df1f4fd-7011-11ec-abfa-7729db260f07 처럼 String 타입으로 인식하지 않는 형태의 응답이 오는 일이 흔한지 궁금합니다.
  - 흔히 볼 수 있는 형태라면 어떤 방식으로 처리하시는지 궁금합니다.
- String(data:encoding:) 을 활용하여 String으로 변환할 수 있었는데, 통신을 수행하는 메서드 내부에서 JSONDecoder를 사용하여 파싱하기 때문에 해당 메서드에 추가로 분기처리가 필요해보였으나, 같은 Data 타입이라 분기처리가 가능할지도 의문입니다...

조언을 얻고 싶은 부분

JSON 파싱을 어디서 해야 할지

기존에는 doDataTask가 데이터 통신만을 진행하여 (Result<Data, Error>) 형태의 completionHandler를 갖고 있었습니다.
현업에서는 dataTask와 JSON파싱을 나눠 놓게 되면, 과정이 번거로워 나누지 않는다는 다른 리뷰어 분의 조언을 듣게 되어 JSON 파싱을 하지 않으면 데이터를 쓰지 못하는 점을 감안해 doDataTask 메서드에서 JSON 데이터를 파싱하는 기능도 처리하게 변경했습니다.
쿠마는 JSON 파싱을 어떻게 처리하시는지 궁금합니다.

확장성

요청하는 메서드들을 API명세에 나와 있는 대로 ProductService 내에 각각 만들었습니다.
메서드 내에서 진행 방식이 유사한 점, API명세에 항목이 추가될 때마다 메서드를 추가해주는 점 등을 이유로 리팩터링을 시도해보았습니다. 하지만 여러 시도 끝에 기존에 작성했던 코드 대로 제출하게 되었습니다.
어떤 방식으로 개선해 볼 수 있을지 조언 부탁드립니다.

폴더링

Extension, Error 를 포함하여 Model, Utility, Service 로 폴더를 나누어보았습니다. 그 외 Mock 객체를 이용한 Test Double 폴더도 만들어서 네트워킹하며 진행한 테스트와 분리했습니다.
현재 폴더와 파일 구조에서 개선할 부분이 있을지 궁금합니다.

- ProductList, Product, Currency, Image, Vendors - 각 타입에 필요한 CodingKey구현

- 요청과 응답처리에 쓰이는 DataTask를 수행하는 doDataTask 구현 - Product 리스트 조회를 위한 retreiveProductList 구현

- ProductService 내 메소드의 파라미터타입을 해당 프로토콜로 변경 - 코드컨벤션 준수를 위한 개행 - 타입명 및 메서드 오기 수정 - JSON파일 에셋에 추가

- 상품 리스트 조회하기 위해 retrieveProductList 메서드에 생략 가능한 파라미터 추가 - doDataTask<Element: Decodable> 제네릭 메서드로 변경 - 메서드 변경으로 인한 test코드 일부 수정 - 불필요한 개행 삭제

- urlRequest 메서드 및 HttpMethod 타입 생성 - URLSession에서 URLSessionProtocol 채택 - Product의 Vendors 타입 생성

- ProductService에서 decode 메서드 분리 - Generic 삭제 후 Result 타입 반환하도록 수정 - DecodeUtility 타입 생성 - Product 내 일부 프로퍼티를 Date 타입으로 받아오기 위해 DateFormatter 구현 - NetworkingError 타입 생성

- ProductService의 doDataTask메서드를 APIService 기본 구현 메서드로 변경 - APIService에 checkNetworkConnection 메서드 기본 구현

- 상품의 secret을 조회하는 retrieveSecretOfProduct 메서드 구현 - URLRequest extension에 addHTTPHeaders 메서드 구현 - UserDefaults를 관리하는 UserDefaultUtility 타입 생성 - 상품의 secret 조회를 요청하기 위한 SecretOfProductRequest 타입 생성 - defaultHeader 프로퍼티 생성

- 상품 정보를 수정하는 modifyProduct 메서드 구현 - ProductModificationRequest 타입 생성

- 상품을 등록하는 registerProduct메서드 구현 - 상품등록 요청에 사용하는 registerProductRequest타입 생성 - APIService에 multipart/form-data 생성을 위한 메서드 구현 - Data타입에 append메서드 구현을 위한 Data+Extension 생성 - DecodeUtility의 Dateformat을 24시간계에 대응가능하도록 수정

- doDataTask 메서드 내에서 JSON 파싱하도록 변경 - 반복되는 URL을 enum에 프로퍼티로 추가 - 상품 수정 요청 시 사용하는 프로퍼티 기본값 삭제, 이니셜라이저 사용하기 위해 변수로 선언 - 코드 컨벤션에 맞추어 개행

leejun6694

숲재, yeha 안녕하세요 : )
step 1 을 구현하느라 고생 많으셨습니다! 코멘트 남긴 부분이 있는데, 확인 부탁드립니다~~!

leejun6694 · 2022-01-09T19:03:10Z

OpenMarket/OpenMarket/Model/Product.swift

+    var identification: Int
+    var url: String
+    var thumbnailUrl: String
+    var succeed: Bool


Bool type 의 변수는 의문문으로 naming 을 해주면 좀 더 자연스러울 수 있습니다 : )

isSuccess 로 고쳐보았습니다!

leejun6694 · 2022-01-09T19:05:57Z

OpenMarket/OpenMarket/Service/APIService.swift

+        completionHandler: @escaping (Result<Type, NetworkingError>) -> Void
+    ) {
+        let task = session.dataTask(with: request) { data, response, error in
+            guard error == nil else {


error 유무부터 처리해주셨네요 bb

leejun6694 · 2022-01-10T19:21:11Z

OpenMarket/OpenMarket/Service/ProductService.swift

+        session: URLSessionProtocol,
+        completionHandler: @escaping ((Result<Product, NetworkingError>) -> Void)
+    ) {
+        let urlString = "\(HTTPUtility.baseURL)\(HTTPUtility.productPath)\(productIdentification)"


반복되는 코드가 있네요. 이를 줄일 수 있는 방법은 없을까요?

leejun6694 · 2022-01-10T19:32:28Z

OpenMarket/OpenMarket/Utility/HTTPUtility.swift

+import Foundation
+
+enum HTTPUtility {
+    static let baseURL: String = "https://market-training.yagom-academy.kr/"


생략 가능한 코드가 있습니다 : )

네! 이 부분 수정했습니다

leejun6694 · 2022-01-10T19:34:40Z

OpenMarket/OpenMarket/Utility/UserDefaultUtility.swift

+    static let KEY_VENDOR_ID = "KEY_VENDOR_ID"
+    static let KEY_VENDOR_PASSWORD = "KEY_VENDOR_PASSWORD"
+
+    func setVendorIdentification(identification: String) {


VendorIdentification 을 set 한다는 의미가 method 이름에서 알 수 있습니다. parameter 명에서 identification 를 다시 알려주고 있네요. 중복되는 부분이 있는 것 같습니다.

레이블에 와일드카드를 추가해서 호출할 때 identification을 중복해서 사용하지 않도록 바꿔보았습니다. 어떤가요?

leejun6694 · 2022-01-10T19:36:20Z

OpenMarket/OpenMarket/Utility/UserDefaultUtility.swift

+    static let KEY_VENDOR_PASSWORD = "KEY_VENDOR_PASSWORD"
+
+    func setVendorIdentification(identification: String) {
+        self.userDefaults.set(identification, forKey: UserDefaultUtility.KEY_VENDOR_ID)


self. 를 생략한 코드도 있고, 이처럼 작성해준 부분도 있습니다.
한가지 스타일로 통일하면, 가독성에 도움이 될 수도 있겠네요 : )

앗, 꼭 필요한 부분을 제외하고 self 지운 것으로 통일해주었습니다!

leejun6694 · 2022-01-10T19:37:07Z

OpenMarket/OpenMarket/Utility/UserDefaultUtility.swift

+
+struct UserDefaultUtility {
+    let userDefaults = UserDefaults()
+    static let KEY_VENDOR_ID = "KEY_VENDOR_ID"


대문자 및 snake 방식을 사용한 이유가 궁금합니다~!

UserDefaults를 사용할 때 프로퍼티명을 snake방식을 사용해야 된다고 알고 있었는데, 잘못된 방식이었습니다.
swift 컨벤션에 맞게 카멜 케이스로 수정하였습니다.

leejun6694 · 2022-01-10T19:37:47Z

OpenMarket/OpenMarket/Utility/UserDefaultUtility.swift

+    }
+
+    func getVendorIdentification() -> String {
+        return self.userDefaults.string(forKey: UserDefaultUtility.KEY_VENDOR_ID) ?? ""


"" 값으로 return 값을 정해줘도 될지 한번 더 고민해보면 좋겠네요...!

이 부분은 return nil로 바꾸어주고, 이 메서드를 사용하는 부분에서 처리해보았습니다!

leejun6694 · 2022-01-10T19:41:21Z

OpenMarket/OpenMarket/Service/ProductService.swift

+        doDataTask(with: request, session: session) { result in
+            completionHandler(result)
+        }
+    }


특정 url, header, value 등 각 api 호출에서 각각 설정해줘야하는 값들이지만, 다시 살펴보면 각 api 에서 중복되는 코드라는 점이 없지 않아 있습니다.
이런 부분을 공통 로직에서 어떻게 다룰 수 있을지 고민해보면, 더 많은 공부가 될 것 같네요 : )

leejun6694 · 2022-01-10T19:55:50Z

4df1f4fd-7011-11ec-abfa-7729db260f07

흔하게 정의되는 response type 은 아니지 않을까 생각됩니다. 다만, API 명세에서 확인할 수 있듯이 string type 은 아닙니다. 현재 구현된 방법은 명세에 맞는가? 라고 물었을 때, 확신의 답을 하진 못할 것 같네요. 말씀주신 것처럼 data type 으로 처리하게 되면, 공통 로직을 해칠 수 있습니다. flag 라던가, 에러 케이스에서 분기를 주는 방법 등 다른 방법들을 생각해볼 수 있겠네요.

JSON 파싱을 어디서 해야 할지

과정이 번거로워진다는 예시는 궁금하네요ㅎㅎ 우리가 어떤 것에 더 집중할지 얘기를 할 수 있을 것 같습니다.
하나의 method 역할을 생각할지, 구현의 번거로움이 생각할지..

확장성

코멘트로도 남겼는데, 생각해보시다가 막히는 점이 있다면 언제든 말씀 부탁드려요 : )

폴더링

잘 나눠주셨고, 파일명도 이해가 편했습니다. 테스트 코드들도 따로 분류하고 작성해주셨네요 👍

- UserDefaultUtility 프로퍼티, 아규먼트 레이블 네이밍 수정 - Image 타입의 succeed프로퍼티 네이밍 수정

- getVendorIdentification, getVendorPassword 수정 - UserError 에러 타입 생성

- getVendorIdentification 메서드가 nil을 반환하도록 수정 - 위의 반환값이 nil일 경우 addHTTPHeaders 메서드가 헤더를 추가하지 않도록 수정 - addValue를 사용하던 부분을 addHTTPHeaders로 통합

- 불필요하게 String 타입을 명시한 부분 삭제 - 불필요한 self 키워드 사용 부분 삭제

leejun6694

좀 더 생각해봐야하는 부분은 step 2 이후에 진행해보아요 : )
고생 많으셨습니다~~!

hayeonhee and others added 26 commits January 3, 2022 16:59

chore: Swiftlint 설치 및 적용

2ae6ad5

feat: JSON 파싱을 위한 타입 구현 #1

716a306

- ProductList, Product, Currency, Image, Vendors - 각 타입에 필요한 CodingKey구현

chore: ProductList타입 프로퍼티명 오기 수정 #1

59f5a3b

feat: 네트워킹에 필요한 URL과 세션을 보관하는 타입 구현 #2

30d4c74

feat: Product 관련 통신기능을 담은 타입 구현 #2

3300662

- 요청과 응답처리에 쓰이는 DataTask를 수행하는 doDataTask 구현 - Product 리스트 조회를 위한 retreiveProductList 구현

test: 테스트를 위한 Mock 객체 구현 #2

000498d

feat: URLSessionProtocol 추가

35de6cf

- ProductService 내 메소드의 파라미터타입을 해당 프로토콜로 변경 - 코드컨벤션 준수를 위한 개행 - 타입명 및 메서드 오기 수정 - JSON파일 에셋에 추가

test: Mock객체 생성하여 데이터 파싱 테스트 추가

b694823

file: Contents.json 파일 추가

81a32f9

feat: 상품 상세조회 메서드 구현

6616af9

- urlRequest 메서드 및 HttpMethod 타입 생성 - URLSession에서 URLSessionProtocol 채택 - Product의 Vendors 타입 생성

test: 상품 상세조회 테스트 구현

2a8e1c8

test: 테스트 코드 수정 및 Mock 테스트 파일 분리 #2

03f168c

feat: APIService 프로토콜 구현 #2

9380ab9

- ProductService의 doDataTask메서드를 APIService 기본 구현 메서드로 변경 - APIService에 checkNetworkConnection 메서드 기본 구현

test: CheckNetworkConnection메서드 테스트 구현

80717b9

test: retrieveSecretOfProduct 메서드 테스트 구현 #2

9972324

feat: deleteProduct메서드 구현 #2

1eb467c

test: DeleteProduct메서드 테스트 구현 #2

4dfa114

feat: 상품 정보를 수정하는 기능 구현 #2

eeb4cbc

- 상품 정보를 수정하는 modifyProduct 메서드 구현 - ProductModificationRequest 타입 생성

test: modifyProduct메서드 테스트 구현 #2

2e64252

test: RegisterProduct메서드 테스트 구현 #2

1e65ef3

test: 로직 변경에 따른 테스트 코드 수정 #2

b09c3d8

leejun6694 requested changes Jan 10, 2022

View reviewed changes

forestjae added 2 commits January 11, 2022 10:45

refactor: 네이밍 수정 #3

a78b8ba

- UserDefaultUtility 프로퍼티, 아규먼트 레이블 네이밍 수정 - Image 타입의 succeed프로퍼티 네이밍 수정

refactor: isSuccess프로퍼티 CodingKey값 추가 #3

dd8ae64

hayeonhee and others added 6 commits January 11, 2022 11:21

refactor: 일부 메서드가 에러를 던지도록 수정 #3

c019a75

- getVendorIdentification, getVendorPassword 수정 - UserError 에러 타입 생성

refactor: 불필요한 코드 삭제 #3

762cdbd

- 불필요하게 String 타입을 명시한 부분 삭제 - 불필요한 self 키워드 사용 부분 삭제

test: ID상수 생성 및 실패메세지 수정 #3

f14d44e

style: 코드 컨벤션 준수를 위한 수정 #3

de1164a

style: 불필요한 주석 삭제#3

643d0d2

leejun6694 approved these changes Jan 11, 2022

View reviewed changes

leejun6694 merged commit cb8ce3c into yagom-academy:4-forestjae Jan 11, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

오픈 마켓 [STEP 1] 숲재, yeha #90

오픈 마켓 [STEP 1] 숲재, yeha #90

hayeonhee commented Jan 8, 2022 •

edited

Loading

leejun6694 left a comment

leejun6694 Jan 9, 2022

hayeonhee Jan 11, 2022

leejun6694 Jan 9, 2022

leejun6694 Jan 10, 2022

leejun6694 Jan 10, 2022

hayeonhee Jan 11, 2022

leejun6694 Jan 10, 2022

hayeonhee Jan 11, 2022

leejun6694 Jan 10, 2022

hayeonhee Jan 11, 2022 •

edited

Loading

leejun6694 Jan 10, 2022

forestjae Jan 11, 2022

leejun6694 Jan 10, 2022

hayeonhee Jan 11, 2022

leejun6694 Jan 10, 2022

leejun6694 commented Jan 10, 2022

leejun6694 left a comment

오픈 마켓 [STEP 1] 숲재, yeha #90

오픈 마켓 [STEP 1] 숲재, yeha #90

Conversation

hayeonhee commented Jan 8, 2022 • edited Loading

고민했던 점

URLSession을 활용한 네트워킹 모델 구현

UserDefaults의 사용

해결이 되지 않은 점

개별 상품의 secret조회 시 디코딩 실패

조언을 얻고 싶은 부분

JSON 파싱을 어디서 해야 할지

확장성

폴더링

leejun6694 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hayeonhee Jan 11, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leejun6694 commented Jan 10, 2022

leejun6694 left a comment

Choose a reason for hiding this comment

hayeonhee commented Jan 8, 2022 •

edited

Loading

개별 상품의 `secret`조회 시 디코딩 실패

hayeonhee Jan 11, 2022 •

edited

Loading