Tercer entregable #39
Conversation
-Pantalla de lista de monedas -Pantalla de detalle
-Create InitApplication class to build database
-HttpLoggingInterceptor -Added User-Agent Header
-Modificaciones de diseño -Corutinas
…and OpenOrdersAdapter. -Move setonClickListener on ListAdapter Item to init on ViewHolder instead of bindItem
|
|
||
| override fun onBindViewHolder(holder: ViewHolder, position: Int) { | ||
| val context: Context = holder.itemView.context | ||
| holder.bindItem(data[position], context) |
There was a problem hiding this comment.
Muy bien, ya cambiaste el RecyclerView.Adapter a ListAdapter. El único cambio necesario ahora es que el ListAdapter no necesita una referencia a una lista, y por tanto, no es necesario el método getItemCount ni la lista 'data' en el constructor.
Para obtener la info en onBindViewHolder se puede cambiar data[position por getItem(position).
| } | ||
| availableBooksVM.availableOrderBookList.observe(viewLifecycleOwner) { | ||
|
|
||
| rvBooks.adapter = AvailableBooksListAdapter( |
There was a problem hiding this comment.
La instancia del adapter no se debe crear dentro de un observer ya que es costoso y puede generar memory leaks. Esta instancia va por fuera del observe, y para actualizar la lista puedes utilizar adapter.submitList(it ?: emptyList())
| } | ||
|
|
||
| override fun onBindViewHolder(holder: ViewHolder, position: Int) { | ||
| holder.bindItem(data[position]) |
There was a problem hiding this comment.
Mismo comentario que en AvailableBooksListAdapter. Puedes cambiar data[position por getItem(position) y eliminar la referencia 'data', eliminar data del constructor y eliminar la implementación de getItemCount
| ): OrderBookResponse | ||
|
|
||
| @GET("available_books") | ||
| fun getAvailableBooksRxJava(): Observable<Response<AvailableBooksResponse>> |
There was a problem hiding this comment.
Similar a como se hizo en corutines, no es necesario tener Response<> alrededor de la respuesta. El tipo puede cambiarse simplemente a Observable
| bookList | ||
| } else { | ||
| Log.i("LocalDataBase", "Getting availableBooks from local database.") | ||
| withContext(Dispatchers.IO) { |
There was a problem hiding this comment.
Este withContext puede quedar a nivel más general cubriendo toda la función, es decir:
getAvailableBooks(): List = withContext(Dispatchers.IO) { ... }
Esto porque tanto consultas locales como consultas a Retrofit deben ejecutarse en el hilo IO.
*Aplica también para los otros métodos en esta clase como getTicker() y getOrderBook()
| private var _selectedOrderBookName = MutableLiveData<String>() | ||
| private var _selectedCoinName = MutableLiveData<String>() | ||
|
|
||
| private var _ticker = MutableLiveData<Ticker>() |
There was a problem hiding this comment.
Estas variables deben ser 'val' en lugar de 'var'
|
|
||
| private val defaultScheduler: Scheduler = Schedulers.io() | ||
|
|
||
| suspend fun getAvailableBooksRxJava() = MutableLiveData<List<AvailableBook>>().apply { |
There was a problem hiding this comment.
No deberías crear un MutableLiveData aqui.
| loadFragment(AvailableBooksFragment()) | ||
| } | ||
|
|
||
| private fun loadFragment(fragment: Fragment) { |
There was a problem hiding this comment.
Con la implementación de navigation component, este método ya no es necesario
| app:layout_constraintStart_toEndOf="@id/img_down" | ||
| app:layout_constraintBottom_toTopOf="@id/ll_bids_asks"/> | ||
|
|
||
| <LinearLayout |
There was a problem hiding this comment.
Como te había comentado previamente, el objetivo de ConstraintLayout es, en la medida de lo posible, evitar layouts internos. Algunos de los LinearLayout que tienes en este archivo se pueden evitar, y colocar las vistas directamente en el constraintLayout.
| import org.junit.Rule | ||
| import org.junit.Test | ||
|
|
||
| class BitsoRepositoryImpTest { |
There was a problem hiding this comment.
Revisa los tests en este archivo. Algunos están fallando por distintas razones, por ejemplo, algunos logs que colocaste en BitsoRepositoryImp (que no se pueden reconocer en un unit test), y algunos métodos que no están mockeados como deleteOpenOrdersFromDatabase(_) en tests como insert order books ...
-Instance adapter outside observer. -Delete Response <> from BitsoApi. -withContext() now covers the entire function. -Delete method with blocking prefix. -AsksEntity and BidsEntity changed forEach to map. -Delete suspend when using RxJava. -Cleaning CompositeDisposable with dispose() on onCleared() method. -Change var to val on ViewModels. -Delete MutableLiveData on AvailableBooksViewModel line 29. -Delete loadFragment function because the navigation component already does the job. -Change LinearLayouts to ConstraintLayouts.
|
Buenas tardes Diego. Subí 2 commits con las correcciones que mencionas. Para que sea mas facil ubicarlas, van corregidas las observaciones a las que les puse un pulgar arriba en tus comentarios. De antemano gracias. |
-Fix to get Asks and Bids from Api
-Add Ktlint to be able to analize code.
-Navigation controller and RxJava2.
-Pruebas unitarias
-Modificaciones de diseño
-Corutinas
-Change RecyclerView.Adapter to ListAdapter on AvailableBooksAdapter and OpenOrdersAdapter.
-Move setonClickListener on ListAdapter Item to init on ViewHolder instead of bindItem