-
Notifications
You must be signed in to change notification settings - Fork 117
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
오픈마켓 II [STEP 1] 웡빙, 보리사랑 #195
Conversation
- 요청 데이터 타입인 ProductRegistration 타입 구현 - 응답 데이터 타입인 ProductDetail 타입 구현 - HTTP Method시 자주 사용될 namespace 추가 - HTTP POST 메서드의 body 생성 코드를 createPostBody 메서드로 분리 - URLSession 타입을 NetworkManager의 전역으로 변경 - MainViewController에서 post 테스트 코드 작성 - URLSession의 dataTask를 createDataTask 메서드로 생성하도록 변경
- 수정사항을 저장하기 위한 ModificationData 구조체 생성 - 수정할 데이터를 RowData로 변환해주는 translateToRowData() 메서드 구현 - RowData를 기반으로 PETCH 요청을 보내는 requestProductModification() 메서드 구현
- 삭제 키를 요청하는 requestDeleteKey 메서드 구현 - 삭제 키를 이용하여 서버에서 Product를 삭제할 수 있는 requestProductDelete메서드 구현 - text/plain 타입의 response도 decode 할 수 있도록 decode 메서드 수정
- 화면 요구사항에 맞게 "ProductSetupView" UI구현 - "ProductSetupViewController" 생성 및 View 등록 - MainViewController에서 + 버튼을 눌렀을 때 이동하도록 구현
- width와 height 지정하는 메서드 extension으로 지정 - ImageVIew를 재사용하기 위해 PickerImageVIew 타입 구현
- Detail Controller에서 UiImagePickerController delegate 구현 - PickerController를 이용하여 주입하기 위해 미리 등록했던 ImageView를 삭제
- 왼쪽의 leftView에 padding view 추가 - height 지정하는 코드 분리
- 게시물의 가격이 Double형태로 들어있을 때 조회가 안되는 오류 발생 - 가격의 타입을 Int 에서 Double로 변경해주어 해결
- 이미지와 텍스트필드에 값이 조건대로 들어가 있으면 done 버튼 터치 시, post수행과 성공alert 표시, 현재 뷰 dismiss - 조건에 부합하지 않은 상태로 done 버튼 터치 시, 경고 alert 표시
- 셀 선택시 ProuctSetupViewController로 이동하게 구현 - Update로 ProuctSetupViewController를 넘어갈 시 수정할 데이터를 미리 등록 - ProductDetail을 HTTP GET 메서드로 가져올 수 있게 NetworkManager타입에 메서드 추가 - ProuctSetupViewController에 productId가 nil인지에 따라 수정과 등록으로 분기하도록 수정 - 수정과 등록에 따라 분기하도록 타이틀과 addItemButton을 등록하는 코드 수정
- 오토레이아웃 오류 수정
- 300KB(307200byte)가 넘어갈 시 300KB보다 작아질 떄까지 0.5배로 압축하도록 변경
- 숫자가 들어가는 TextField는 numberpad로 타이핑하도록 키보드 타입 변경 - currency가 USD일 경우 decimalPad로 다시 변경
- 코드 작성자 수정 - 상속받지 않는 class에 final 키워드 부여 - HTTP Method 테스트 주석 삭제
- ProductSetupViewController에서 pop 될 때 Keyboard Responder 등록을 해제함.
- 키보드 레이아웃이 textView 를 가리는 현상 제거 - 키보드를 내리는 inputAccessoryView 추가 - 오토레이아웃 제약 오류 수정 - 코드 정리 및 컨벤션 통일
return button | ||
}() | ||
|
||
private var rootViewController: UIViewController? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
뷰가 vc를 가지는 건 최대한 피해야 합니다.
그리고 있더라도 retain cycle이 돌기 때문에 메모리 누수 발생하므로 weak var 로 선언하셔야 합니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
내부에 vc를 프로퍼티로 가지고 있는 코드를 제거 해 보았습니다.
최초에 프로퍼티로 vc를 저장한 이유가
init(_ rootViewController: UIViewController) {
super.init(frame: .null)
// view setup code
}
이런 이니셜 라이저를 사용하는데 ProductSetupView를 생성하는 뷰 컨트롤러를 이니셜라이저로부터 주입받았으니 이를 가지고 있는 편이 내부에서 부모 뷰(뷰컨)의 frame에 맞게 constraint를 잡기 편할 것이라고 생각했기 때문입니다.
말씀 주셔서 프로퍼티로 가지고 있는 vc를 제거해보니 어차피 주입받은 전달인자로 초기에 레이아웃을 잡아서 문제가 없었는데 이런식으로 하는게 맞을까요?
아니면 기본 이니셜라이저중에 override init(frame: CGRect)
이것을 이용해서 ProductSetupView를 호출하는 VC가 자신의 safeAreaLayoutGuides
를 넘기면서 여기에 맞추는걸 말씀하신걸까요...?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
말씀 주셔서 프로퍼티로 가지고 있는 vc를 제거해보니 어차피 주입받은 전달인자로 초기에 레이아웃을 잡아서 문제가 없었는데 이런식으로 하는게 맞을까요?
네네, 레이아웃 잡는데 필요한 거라면 굳이 vc를 프로퍼티로 가지지 않아도 되서 이렇게 코멘트 드렸습니다!
그리고 추가적으로 보통 뷰가 자신의 상위객체(자신을 프로퍼티로 가지는 객체)을 프로퍼티로 갖는 구조는
참조 사이클이 돌 수 있는 좋지 않은 패턴이기 때문에 최대한 피해야 한다고 말씀 드렸어요~~
} | ||
|
||
private func keyboardTypeSetup() { | ||
// productNameTextField.keyboardType = .default |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
주석 제거해주세요 ㅎㅎ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
제거 해보았습니다...!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다!!
더 추가적으로 리팩토링이 필요한건 모든 화면이 mvc 패턴대로
1) 서버요청 후 viewController 에서 모델에 응답값 전달
2) 모델에 응답값 업데이팅 후 노티피케이션(or rx, combine) 로 뷰컨에 모델값 변했다고 노티
3) 뷰컨트롤러는 모델 을 옵저빙하다가 노티 받으면 변경된 모델 값 기준으로 뷰 업데이트
이런 패턴이 보이도록 리팩토링 하시면 좋을 것 같아요 ㅎㅎ (참고예제: https://github.com/godrm/swift-pokergameapp/tree/Network)
그럼 고생 많으셨습니다.
일단 메모리 누수만 제거 하시고 다음 단계 진행하시면 될 것 같아요! 👏👏👏
사용자가 터치한 곳의 y값을 알아내는 방법은 위 링크를 보시면 됩니다! |
callback 처리저는 나눈다면 rx를 써서 나눌 것 같아요(rx를 안다면). onNext로 전달 후에 다음 api 요청을 보낼 것 같습니다. 추가로 PR 내용 잘 넣어주셔서 이해하기 훨씬 수월 했습니다. 클래스 다이어그램도 잘 그리셨네요 👍 |
- ProductSetupView가 내부에 VC를 가지는 부분을 제거
- MainViewController 에서 관리하던 모델을 분리 - ProductListManager 클래스를 생성하여 안에서 모델의 업데이트를 관리하도록 구현 - 모델이 변경되었을 때 Notification 을 보내서 MainViewController 에서 업데이트를 하도록 구현
@@ -112,17 +115,26 @@ final class MainViewController: UIViewController { | |||
} | |||
|
|||
private func fetchData() { | |||
manager.requestProductPage(at: 1) { [weak self] productList in | |||
self?.productListManager.fetch(list: productList) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
받아온 모델값을 업데이팅하는것이니 네이밍은 fetch 가 아니라 update 가 나을 듯 합니다 :)
오픈마켓 STEP3 PR
tags:
PR
안녕하세요! 웡빙, 보리사랑 입니다. ( @wongbingg, @yusw10 )
오픈마켓 II STEP1 PR 보내드립니다.
잘부탁 드립니다 제이슨! @ehgud0670 🫡
Step 3 기능 구현
1. HTTP 메서드 구현
createDataTask()
메서드 구현 (재사용)createPostBody()
메서드를 통해 httpBody 를 생성 후 대입createDataTask()
메서드에 request 대입 후 resumetranslateToRowData()
메서드를 통해 수정할 데이터를 rowData로 변환 후 대입createDataTask()
메서드에 request 대입 후 resumecreateDataTask()
메서드에 request 대입 후 resumecreateDataTask()
메서드에 request 대입 후 resume2. UI 구현
3. UI와 HTTP 메서드 연결
+
버튼에 POST 메서드 연결4. 이미지 압축
Image.jpegData(compressionQuality:)
메서드를 통해 0.2배율로 압축UML
Class Diagram(Step2 PR 기준)
궁금한 점 / 고민한 점
call back 처리
Delete 메서드를 테스트 하면서 느낀점으로,
과 같이 completion Handler가 여러번 호출해야 함을 확인했습니다. 굉장히 가독성이 떨어지는 것 같은데(구현부를 확인하기 위해 파일을 여러번 확인해야해서) 이런식으로 completion 핸들러가 계속해서 호출하는 형태에서는 어떻게 처리하시는지 궁금했습니다.
키보드 처리
텍스트 입력 중 키보드가 콘텐츠를 가리지 않도록 합니다.
라는 명세서 요구사항이 있어 이를 해결하려고 했으나 아직 해결중에 있습니다. 다음과 같은 방법으로 시도 해보았습니다.그리고 아직 정확한 로직이 이해가 되지 않는 부분이 있습니다.
resignFirstResponder
를 통하여 키보드를 사라지게 하였는데, 이 때 사라지는 로직에 키보드가 한번 show 된 뒤에 사라지고 있습니다.다음과 같이 키보드가 나타나고 사라질 때를 Notification에 등록하고 로그를 찍어 보았는데
![](https://camo.githubusercontent.com/a132cacb0ece567ae8cd2301a7972cf9fe6e3bc71849da9af28bc6f75082d85c/68747470733a2f2f692e696d6775722e636f6d2f63566b565a47722e706e67)
위와 같이 return 버튼을 누른 순간 resignFirstResponder를 처리하는 시점 직전에 키보드가 한번 올라오는 로직이 실행되는 것 같은데 이유를 확인 중에 있습니다.
트러블 슈팅
text/plain 타입의 Response 디코딩이슈 -> 해결
오픈마켓 서버에서 삭제 secret을 조회하는 POST 메서드는 text/plain 타입으로 response data가 오게 되어있습니다. 하지만 저희가 구현한 decoding 메서드는 application/json 타입의 데이터만 처리할 수 있게 구현되어 있었고, 다음과 같이 디코드 메서드 앞에 코드를 추가하여 해결할 수 있었습니다.