[스캇] 3, 4단계 쇼핑 장바구니 제출합니다. #39
Conversation
app/src/main/java/woowacourse/shopping/presentation/productdetail/ProductDetailDialog.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/woowacourse/shopping/presentation/productdetail/ProductDetailDialog.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/woowacourse/shopping/data/cart/CartRepositoryImpl.kt
Outdated
Show resolved
Hide resolved
| override fun getProductsWithRange(start: Int, range: Int): List<Product> { | ||
| var newProducts: List<Product> = emptyList() | ||
| val thread = Thread { | ||
| val client = OkHttpClient() | ||
| val host = "http://localhost:$PORT/" | ||
| val path = "products?start=$start&range=$range" | ||
| val request = Request.Builder().url(host + path).build() | ||
| val response = client.newCall(request).execute() | ||
| val body = response.body?.string() ?: return@Thread | ||
| newProducts = parseResponse(body) | ||
| } | ||
| thread.start() | ||
| thread.join() | ||
| return newProducts | ||
| } |
There was a problem hiding this comment.
별도 Thread를 통해 요청하도록 잘 만들어주셨네요.
다만 매번 요청을 할 때마다 Thread를 만들게되면 굉장히 큰 부하가 걸리게됩니다.
HTTP 요청에 대해서 이미 잘 만들어진 라이브러리들을 활용해볼까요? (OkHttp, Retrofit)
There was a problem hiding this comment.
개인적으로는 Executor를 이용하여 직접 고정된 Thread Pool을 만들어 보는 것도 공부하는 단계에서는 굉장히 큰 도움이 될 거라 생각합니다.
이 방식은 이제 현업에서는 legacy로 여겨지지만 OkHttp와 라이브러리의 내부 코드에서는 위와 같이 Thread Pool을 만들어서 Thread를 재사용하기 때문에 내부 동작 방식을 이해하는데 도움이 될 수 있기 때문이에요.
There was a problem hiding this comment.
별도의 스레드를 생성하지 않고, Retrofit 이나 OkHttp 가 자체적으로 스레드를 생성하려면, enqueue 를 활용하여 비동기적으로 작업을 수행을 해야 하는데, 이 때 콜백을 어떻게 처리해야 할지 감이 안잡히네요... 지금 구조에서는 동기적으로 코드를 작성하였기 때문에, Repository 에서 데이터를 다 받아오기 까지 기다리고, 다 받아오면 Domain 객체를 반환하여, 그 객체를 Presenter 가 이용하는 방식입니다. 하지만 비동기 방식으로 짜게되면, Repository에서 Call<List>를 반환하고, 그 반환 값을 Presenter 가 enqueue 를 통해 처리해야 구조로 바뀌게 되야 할것 같습니다. 그런데 이렇게 되면, 모든 비동기를 적용한 View 코드에서 runOnUiThread 가 추가되어야 하고, Repository가 객체를 전달하는것이 아니라 Call로 감싼 객체를 전달하는 것이 옳은가에 대해서도 의문이 드네요. 다른 좋은 방법이 있을까요??
There was a problem hiding this comment.
여러가지 방법이 있습니다!
- 콜백함수를 넘겨서 처리
- ExecutorService, Future API를 활용하여 Call 대신 Future or 직접 만든 객체를 반환
- RxJava 활용
- kotlinx.coroutines 활용
여러 방법을 써놓았는데 1번에서 4번까지 모두 조금씩 개선되었던 안드로이드의 역사입니다.
1번에서 일일히 콜백을 넘겨서 처리하다가 불편하니 값이 추후에 가지게 되는 Future를 쓰게 됩니다.
그러다가 좀 더 본격적으로 옵저버 패턴으로 구현한 RxJava가 나오고 reactive programming 패러다임이 인간의 사고방식과 많이 달라서 기존의 절차형 프로그래밍으로 돌아간 coroutines이 나온 것까지가 현재 상황입니다.
현재는 연습하는 단계이니 1번인 콜백함수를 직접 넘겨서 불편함을 느껴보시는 것도 좋을 것 같아요!
There was a problem hiding this comment.
일일이 콜백함수를 넘기는 과정... 상당히 힘들었네요! 그래도 이 과정이 앞으로 많은 도움이 될 것 같습니다!
app/src/main/java/woowacourse/shopping/presentation/binding/ViewBinding.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/woowacourse/shopping/presentation/binding/ViewBinding.kt
Outdated
Show resolved
Hide resolved
| private val _loadedCartProducts = SafeMutableLiveData(initCartProductList) | ||
| override val loadedCartProducts: SafeLiveData<CartProductInfoList> get() = _loadedCartProducts | ||
|
|
||
| private val _pageProducts = SafeMutableLiveData(initCartProductList) | ||
| override val pageProducts: SafeLiveData<CartProductInfoList> get() = _pageProducts |
There was a problem hiding this comment.
observe()를 호출하지 않으므로 LiveData로 선언할 필요가 없습니다. 단순히 var로 선언하는 것으로도 충분하지 않을까요?
또한 MVP 이기 때문에 View에서 해당 값들을 참조하는 대신 Presenter에서 사용하여 View에 어떤 것을 그릴지를 명령해야 합니다.
Contract에서 loadedCartProducts, pageProducts를 제거해보고 View에 명령하도록 만들어보세요.
아마 현재 구현된 방식이 더 편하고 가독성이 좋다고 느껴지실텐데요. (저도 그렇긴 합니다 😅)
그 이유는 LiveData와 DataBinding을 활용하는 방식은 MVVM 패턴이기 때문이에요.
MVP를 구현하시다가 더 편하고 좋은 방법을 찾다가 자연스레 MVVM으로 구현하시게 된 것 같아요.
하지만 이번 미션에서는 MVP로 구현하는게 목표이므로 가독성이 안좋아지더라도 MVP로 변경해보고 MVP에 대한 단점을 마음속에 깊이 기억해두시면 좋겠어요.
There was a problem hiding this comment.
이번에 순수 MVP로 구현하면서 느낀점은, MVP 로 구현하면 테스트가 편해지고, 비즈니스 로직,데이터영역과 뷰 영역을 완전히 분리한다는 점에서 추후 리팩토링이나 확장성에 대해서는 이점이 있을 것 같습니다. 하지만 View 하나당 Presenter와 Contract 가 필요해서 클래스 양이 많아지고, 불필요한 코드가 많아지는 느낌을 받았고, 특히 RecyclerView의 ViewHolder의 뷰를 접근하고자 할 때 상당히 불편했던 것 같습니다...(LiveData 를 이용하면 편했었는데 말이죠...)
There was a problem hiding this comment.
추가적으로, CustomView에 대해 MVP를 적용한 것 처럼, ViewHolder에 대한 ViewHolderPresenter 를 만들어서 관리하는 건 MVP 패턴에서 어긋나는 부분일까요?
There was a problem hiding this comment.
이번에 순수 MVP로 구현하면서 느낀점은, MVP 로 구현하면 테스트가 편해지고, 비즈니스 로직,데이터영역과 뷰 영역을 완전히 분리한다는 점에서 추후 리팩토링이나 확장성에 대해서는 이점이 있을 것 같습니다. 하지만 View 하나당 Presenter와 Contract 가 필요해서 클래스 양이 많아지고, 불필요한 코드가 많아지는 느낌을 받았고, 특히 RecyclerView의 ViewHolder의 뷰를 접근하고자 할 때 상당히 불편했던 것 같습니다...(LiveData 를 이용하면 편했었는데 말이죠...)
정확히 이해해주셨네요 👍
추가적으로, CustomView에 대해 MVP를 적용한 것 처럼, ViewHolder에 대한 ViewHolderPresenter 를 만들어서 관리하는 건 MVP 패턴에서 어긋나는 부분일까요?
ViewHolder는 말그대로 View를 Hold하고 있는 객체입니다.
단순히 메모리를 아끼기 위해 View를 재활용하기 위한 객체이기 때문에 ViewHolder에서 Presenter를 쓰게 되면 객체의 역할을 넘어서게 됩니다.
app/src/main/java/woowacourse/shopping/presentation/common/CounterView.kt
Outdated
Show resolved
Hide resolved
|
안녕하세요 토리! 스캇입니다! |
| override fun getProductsWithRange(start: Int, range: Int): List<Product> { | ||
| var newProducts: List<Product> = emptyList() | ||
| val thread = Thread { | ||
| val client = OkHttpClient() | ||
| val host = "http://localhost:$PORT/" | ||
| val path = "products?start=$start&range=$range" | ||
| val request = Request.Builder().url(host + path).build() | ||
| val response = client.newCall(request).execute() | ||
| val body = response.body?.string() ?: return@Thread | ||
| newProducts = parseResponse(body) | ||
| } | ||
| thread.start() | ||
| thread.join() | ||
| return newProducts | ||
| } |
There was a problem hiding this comment.
여러가지 방법이 있습니다!
- 콜백함수를 넘겨서 처리
- ExecutorService, Future API를 활용하여 Call 대신 Future or 직접 만든 객체를 반환
- RxJava 활용
- kotlinx.coroutines 활용
여러 방법을 써놓았는데 1번에서 4번까지 모두 조금씩 개선되었던 안드로이드의 역사입니다.
1번에서 일일히 콜백을 넘겨서 처리하다가 불편하니 값이 추후에 가지게 되는 Future를 쓰게 됩니다.
그러다가 좀 더 본격적으로 옵저버 패턴으로 구현한 RxJava가 나오고 reactive programming 패러다임이 인간의 사고방식과 많이 달라서 기존의 절차형 프로그래밍으로 돌아간 coroutines이 나온 것까지가 현재 상황입니다.
현재는 연습하는 단계이니 1번인 콜백함수를 직접 넘겨서 불편함을 느껴보시는 것도 좋을 것 같아요!
| private val _loadedCartProducts = SafeMutableLiveData(initCartProductList) | ||
| override val loadedCartProducts: SafeLiveData<CartProductInfoList> get() = _loadedCartProducts | ||
|
|
||
| private val _pageProducts = SafeMutableLiveData(initCartProductList) | ||
| override val pageProducts: SafeLiveData<CartProductInfoList> get() = _pageProducts |
There was a problem hiding this comment.
이번에 순수 MVP로 구현하면서 느낀점은, MVP 로 구현하면 테스트가 편해지고, 비즈니스 로직,데이터영역과 뷰 영역을 완전히 분리한다는 점에서 추후 리팩토링이나 확장성에 대해서는 이점이 있을 것 같습니다. 하지만 View 하나당 Presenter와 Contract 가 필요해서 클래스 양이 많아지고, 불필요한 코드가 많아지는 느낌을 받았고, 특히 RecyclerView의 ViewHolder의 뷰를 접근하고자 할 때 상당히 불편했던 것 같습니다...(LiveData 를 이용하면 편했었는데 말이죠...)
정확히 이해해주셨네요 👍
추가적으로, CustomView에 대해 MVP를 적용한 것 처럼, ViewHolder에 대한 ViewHolderPresenter 를 만들어서 관리하는 건 MVP 패턴에서 어긋나는 부분일까요?
ViewHolder는 말그대로 View를 Hold하고 있는 객체입니다.
단순히 메모리를 아끼기 위해 View를 재활용하기 위한 객체이기 때문에 ViewHolder에서 Presenter를 쓰게 되면 객체의 역할을 넘어서게 됩니다.
| object : Callback<List<ProductDataModel>> { | ||
| override fun onResponse( | ||
| call: Call<List<ProductDataModel>>, | ||
| response: Response<List<ProductDataModel>>, | ||
| ) { | ||
| val productDataModels = response.body() | ||
| if (productDataModels != null) onSuccess(productDataModels.map { it.toDomain() }) | ||
| } | ||
|
|
||
| override fun onFailure(call: Call<List<ProductDataModel>>, t: Throwable) { | ||
| Log.d("HttpError", t.message.toString()) | ||
| } | ||
| }, |
There was a problem hiding this comment.
enqueue() 함수로 넘길 Callback 구현체가 반복되므로 onFailure()를 미리 구현해놓은 클래스를 하나 만들어두어도 좋겠네요!
안녕하세요 토리! 스캇입니다!
저번 1,2 단계 리뷰 정말 많은 도움이 되었습니다! 감사합니다!
이번 미션에서는 LiveData를 이용한 상태관리와, Repository 패턴을 이용하는 것에 많은 집중을 하였습니다.
이번 리뷰에서는 Repository 패턴이 잘 적용 되었는지에 대해 리뷰를 받고싶고, LiveData와 ViewBinding을 이용하면서 도메인과 VIew 부분이 많이 혼용된 느낌이 드는데, 이것에 대해서도 리뷰를 받고싶습니다.
또한, 많은 화면에서 사용되는 Counter 에대해서 CounterView 라는 Custom Layout 을 적용하였습니다. CounterView의 CounterPresenter는 숫자를 올리고 내리는 책임만 가지게 해놓았고, 다른 Activity Presenter 와 서로 협력하는 구조로 작성해 보았습니다.
이 부분에 대해서도 어떤 문제가 있을지 궁금합니다!
이번 리뷰도 잘 부탁드립니다!!