[Neo, JEJE] Detail 뷰 기능 구현, OAuth 뷰 기능 구현 #63
Conversation
Detail View and MVVM 구조 개선
Feat: 주문완료시 알림 추가
There was a problem hiding this comment.
- 함수가 외부에 열려있다면 타이밍과 횟수를 컨트롤 할 수 없다고 생각하고 작성해야합니다.
여러번 혹은 무작위로 불렸을때 원하는대로 동작할지 고민해보면 좋을 것 같네요. - 함수 전체가 main queue로 던지는 건 1번과 연관되어 좋지 않습니다. 현재 호출되는 queue가 어떤건지 알 수 없기때문에 아는 곳에서 컨트롤해주는게 맞습니다.
- 현재 폴더 및 파일 구조를 앱 전체 레이어로 나누지 말고 화면 하나당 레이어로 나눠보세요. 안보이던게 보일 수도 있습니다. 구조가 정리가 안될땐 관계도를 그림으로 시각화해서 분석해보시기 바랍니다.
| self.point = "test" | ||
| self.deliveryInfo = "test" | ||
| self.deliveryFee = "test" | ||
| self.detailSection = [] |
There was a problem hiding this comment.
여기 죄다 test 라는 값이 입력된게 코드만 보면 개발이 완료되지 않은것 같네요.
|
|
||
| func requestResource(path: Path, method: HTTPMethod) -> AnyPublisher<SideDishes, NetworkError> { | ||
| func requestResource<T, U: Decodable>(path: T, method: HTTPMethod, deocdeType: U.Type) -> AnyPublisher<U, NetworkError> { |
There was a problem hiding this comment.
decodeType 파라미터는 필요없어보이는데 확인한번 해보세요.
There was a problem hiding this comment.
SideDishUseCase 보시면 get을 요청하는 Decodable 타입이 다르기 때문에 decodeType 타입이 필요합니다.
There was a problem hiding this comment.
return type으로 타입을 확정지어주기 때문에 필요 없어보입니다. type 관련된걸 protocol 부터 전부 빼보세요
There was a problem hiding this comment.
이해가 잘 안됩니다.ㅠㅠ
// SideDishUseCase.swift
func requestSideDishes(path: Menu) -> AnyPublisher<SideDishes, NetworkError> {
return networkManager.requestResource(path: path, method: .get, deocdeType: SideDishes.self)
}
요청 할 경로, 메서드 , 디코드 타입을 전달해주는데
// NetworkManager.swift
func requestResource<T, U: Decodable>(path: T, method: HTTPMethod, deocdeType: U.Type) -> AnyPublisher<U, NetworkError> {
.....
return request(urlRequest: urlRequest, type: deocdeType)
}
만들어준 urlRequset 와 매개변수로 받은 decode.type을 request함수에 전달
// NetworkManager.swift
private func request<T : Decodable>(urlRequest: URLRequest, type : T.Type) -> AnyPublisher<T, NetworkError> {
return URLSession.shared.dataTaskPublisher(for: urlRequest)
.....
return Just(data)
.decode(type: T.self, decoder: JSONDecoder())
....
}
매개변수로 받은 제네릭 decode.type을 가지고 네트워크 데이터를 디코드 후에 해당 타입으로 리턴을 시켜줍니다
디코더 타입을 지정해주기 때문에 UseCase에서는 확정된 타입을 return 해 줄 수 있었습니다
- 디코더를 해줄 때 어떤 타입으로 디코드 할지 결정해야하기 때문에 필요 하다고 생각합니다
There was a problem hiding this comment.
func requestSideDishes(path: Menu) -> AnyPublisher<SideDishes, NetworkError> {
return networkManager.requestResource(path: path, method: .get)
}
// NetworkManager.swift
func requestResource<T, U: Decodable>(path: T, method: HTTPMethod) -> AnyPublisher<U, NetworkError> {
.....
return request(urlRequest: urlRequest)
}
// NetworkManager.swift
private func request<T : Decodable>(urlRequest: URLRequest) -> AnyPublisher<T, NetworkError> {
return URLSession.shared.dataTaskPublisher(for: urlRequest)
.....
return Just(data)
.decode(type: T.self, decoder: JSONDecoder())
....
}
이런걸 말한건데, 혹시 안되나요?
There was a problem hiding this comment.
됩니다..
이렇게 할 수 있다는걸 처음 봤습니다.. 머리가 띵하네요...
제 생각에는 디코더 할때는 타입을 정해줘야 한다는 생각에 계속 머물렀는데 디코더 타입을 정해주지 않아도 알아서 타입에 따라 데이터가 나올수 있다는게 이게 말이 되나 싶어요
제가 더 많이 고민 해 봤어야 했는데 디코더가 필요하다는 생각에 머물고 계속 주장했던 제가 민망하고 부끄럽네요
이렇게도 될 수 있다는것을 알려주셔서 정말 감사드립니다.
There was a problem hiding this comment.
이렇게 배우는거죠 ㅎㅎ
여기선 리턴 타입을 통해 타입을 확정시켰다고 보면 됩니다.
| return fileManager.fileExists(atPath: url) | ||
| } | ||
|
|
||
| private func createFile(path: String, data: Data?) { |
There was a problem hiding this comment.
fileExists, createFile 함수는 불필요해보입니다. 어차피 fileManager 를 공유하고 있기때문에 사용하는곳에서 바로 사용하면 될 것 같네요
| } | ||
|
|
||
| func amountConfigure(amount: String, total: String) { | ||
| DispatchQueue.main.async { [weak self] in |
There was a problem hiding this comment.
이 함수가 불릴때 무조건 main queue로 강제하는건 바람직하지 않아보입니다.
이 함수가 백그라운드에서만 불린다는 보장이 없기 때문입니다. 호출하는 쪽에서 그 타이밍을 정확히 알기때문에 queue 컨트롤은 타이밍을 아는 쪽에서 하는게 더 좋을 것 같습니다.
There was a problem hiding this comment.
UI를 바꾸는 스레드는 메인에서 해야하기 때문에 main queue로 강제 했는데 이 함수를 호출하는 곳에서 queue를 결정해주는게 바람직한건가요??
There was a problem hiding this comment.
UI를 바꾸는 스레드는 메인에서 하는게 맞지만, 이 함수를 호출하는 순간이 main 스레드인지 혹은 다른 스레드인지 알지 못하기때문에 무조건 메인 스레드로 확정하는 것은 바람직하지 않다는 의미입니다.
| images.forEach { [unowned self] (imageURL) in | ||
| let imageView = UIImageView() | ||
| imageView.translatesAutoresizingMaskIntoConstraints = false | ||
| self.thumbnailStackView.addArrangedSubview(imageView) |
There was a problem hiding this comment.
thumbImageLoad함수가 불릴때마다 stackView에 계속 쌓이는 형태인데 여러번 호출되면 어떨까요? 그 부분에 대한 컨트롤이 취약해보입니다.
원하는 동작이 맞다면 함수명이 변경되어야할거 같아요. add 같은 동작을 명시적으로요.
There was a problem hiding this comment.
여러번 호출되어도 문제 없이 쌓이는데 어떤 부분에 대한 컨트롤이 취약한지 잘 모르겠습니다
어떤 컨트롤에 대한 취약인가요??
현재는 제가 원했던 동작인데 문제 없이 동작합니다, 이름은 수정하도록 하겠습니다
There was a problem hiding this comment.
외부에서 호출할때 thumbnailStackView 에 뷰가 계속 추가되는 동작(addArrangedSubview)을 원하는게 맞다면, 함수 이름만 개선되면 문제 없을 것 같네요.
|
|
||
| private var detailItem : Item! | ||
| private var cancellable = Set<AnyCancellable>() | ||
| private var detailViewModel: DetailViewModelProtocol! |
There was a problem hiding this comment.
dependInjectionViewModel라는 함수에서 주입을 받는데 !로 관리하는건 위험합니다. 주입받기 전에는 무조건 오류가 나기때문입니다. 이는 함수호출에 순서를 부여해야하고 관리하기 힘든 포인트가 됩니다. 객체의 생성시점에 주입을 하는게 일반적이고, 그게 안된다면 옵셔널로 관리를 해야합니다
| bind() | ||
| } | ||
|
|
||
| func bind() { |
There was a problem hiding this comment.
외부에서 사용되는 함수인가요? 바로 위 viewDidLoad에서 부르고 있어서 바인드가 여러번 불리는지 궁금하네요
There was a problem hiding this comment.
DetailViewController 내부 한곳에서만 사용되고 있습니다 private를 빼먹었습니다
| } | ||
|
|
||
| private func request(with detailHash: String) { | ||
| sideDishUseCase.requestItemDetails(detailHash: detailHash) |
There was a problem hiding this comment.
detailViewModel에 sideDish의 usecase가 있는건 좀 이상해보입니다.
(온라인 반찬서비스 ) Side-dish
주요 구현 목록
Study keywords
고민 및 해결
OAuth를 이용한 깃헙 로그인을 구현하기 위해 Octokit 라이브러리를 활용했습니다. 라이브러리 api를 활용해 accessToken을 얻어 accessToken을 이용해 TokenConfiguration객체를 생성했지만 토큰을 이용한 로그인이 지속적으로 401 에러를 뱉어냈습니다. 공식문서에 나와있는 안내대로 구현했지만 나타나는 오류가 이를 해결하기 막막했습니다. 후에 TokenConfiguration의 값을 출력해보니 optional이 unwrapped 됐음에도 불구하고 다시 accessToken 옵셔널로 바뀌는 것을 확인했습니다. 이를 제거해주니 정상적인 로그인이 가능했습니다.🤓
앱을 지우고 다시 설치했을때 캐시가 아닌 네트워크에서 받아오는 디테일뷰의 이미지 로드 순서가 올바르지 않게 오는걸 확인하고 이미지를 출력해주는 로직을 변경하였습니다 미리 이미지뷰를 생성해두고 네트워크 요청을 통해 이미지 순서를 보장할 수 있게 되었고 이미지가 불러오지 못한 경우
해당 이미지 뷰를 Hidden 처리 하도록 하였습니다!
고민은 Api 키를 내부에서 선언에서 사용하는게 매우 안좋은 방법이란걸 알고 있습니다.
현재 해당문제를 해결하려고 plist에 넣어서 사용했지만 이마저도 좋지 않은 방법이라 생각합니다
제가 생각하는 Api 키 관리는 키를 관리하는 서버를 통해 받아오고 처리하는게 좋다고 생각하는데
실무 상에서는 API키를 어떻게 관리하는지, 협업하는데 키를 어떻게 공동으로 소유하는지 궁금합니다