-
Notifications
You must be signed in to change notification settings - Fork 0
Refactoring arch #1
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,8 +5,8 @@ import android.view.ViewGroup | |
| import android.widget.Filter | ||
| import android.widget.Filterable | ||
| import androidx.recyclerview.widget.RecyclerView | ||
| import com.dhruvdroid.data.Content | ||
| import com.dhruvdroid.sampleott.R | ||
| import com.dhruvdroid.sampleott.data.Content | ||
| import com.dhruvdroid.sampleott.databinding.MovieItemBinding | ||
|
|
||
| // | ||
|
|
@@ -15,7 +15,7 @@ import com.dhruvdroid.sampleott.databinding.MovieItemBinding | |
| class ContentListAdapter(val list: MutableList<Content>) : | ||
| RecyclerView.Adapter<ContentListAdapter.MovieViewHolder>(), Filterable { | ||
|
|
||
| private lateinit var listFiltered: MutableList<Content> | ||
| private var listFiltered: MutableList<Content> | ||
|
|
||
| init { | ||
| listFiltered = list | ||
|
|
@@ -33,7 +33,7 @@ class ContentListAdapter(val list: MutableList<Content>) : | |
| inner class MovieViewHolder(private val viewBinder: MovieItemBinding) : | ||
| RecyclerView.ViewHolder(viewBinder.root) { | ||
|
|
||
| fun viewBinding(data: Content) { | ||
| fun viewBinding(data: com.dhruvdroid.data.Content) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. add in file level import |
||
| viewBinder.title.text = data.name | ||
| viewBinder.movieCard.setBackgroundResource(getDrawableResource(data.posterImage)) | ||
| } | ||
|
|
@@ -82,7 +82,7 @@ class ContentListAdapter(val list: MutableList<Content>) : | |
| return listFiltered.size | ||
| } | ||
|
|
||
| fun updateList(content: List<Content>) { | ||
| fun updateList(content: List<com.dhruvdroid.data.Content>) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. add in file level import |
||
| this.list.addAll(content) | ||
| notifyDataSetChanged() | ||
| } | ||
|
|
@@ -94,7 +94,7 @@ class ContentListAdapter(val list: MutableList<Content>) : | |
| listFiltered = if (charString.isEmpty()) { | ||
| list | ||
| } else { | ||
| val filteredList: MutableList<Content> = ArrayList() | ||
| val filteredList: MutableList<com.dhruvdroid.data.Content> = ArrayList() | ||
| for (item in list) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a very old school and entry level developer approach... Please start thinking more functional. Can be written in a single line. |
||
| if (item.name.toLowerCase().contains(charString.toLowerCase())) { | ||
| filteredList.add(item) | ||
|
|
@@ -109,7 +109,7 @@ class ContentListAdapter(val list: MutableList<Content>) : | |
| } | ||
|
|
||
| override fun publishResults(charSequence: CharSequence, filterResults: FilterResults) { | ||
| listFiltered = filterResults.values as MutableList<Content> | ||
| listFiltered = filterResults.values as MutableList<com.dhruvdroid.data.Content> | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. add file level import |
||
| notifyDataSetChanged() | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,22 +6,21 @@ import androidx.recyclerview.widget.DiffUtil | |
| import androidx.recyclerview.widget.ListAdapter | ||
| import androidx.recyclerview.widget.RecyclerView | ||
| import com.dhruvdroid.sampleott.R | ||
| import com.dhruvdroid.sampleott.data.Content | ||
| import com.dhruvdroid.sampleott.databinding.MovieItemBinding | ||
|
|
||
| // | ||
| // Created by Dhruv on 15/08/20. | ||
| // | ||
| class MovieListAdapter : ListAdapter<Content, MovieListAdapter.MovieViewHolder>(DIFF_CHECK) { | ||
| class MovieListAdapter : ListAdapter<com.dhruvdroid.data.Content, MovieListAdapter.MovieViewHolder>(DIFF_CHECK) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. add file level import |
||
|
|
||
| companion object { | ||
| private val DIFF_CHECK = object : DiffUtil.ItemCallback<Content>() { | ||
| override fun areItemsTheSame(oldItem: Content, newItem: Content): Boolean { | ||
| private val DIFF_CHECK = object : DiffUtil.ItemCallback<com.dhruvdroid.data.Content>() { | ||
| override fun areItemsTheSame(oldItem: com.dhruvdroid.data.Content, newItem: com.dhruvdroid.data.Content): Boolean { | ||
| return oldItem.id == newItem.id | ||
| } | ||
|
|
||
|
|
||
| override fun areContentsTheSame(oldItem: Content, newItem: Content): Boolean { | ||
| override fun areContentsTheSame(oldItem: com.dhruvdroid.data.Content, newItem: com.dhruvdroid.data.Content): Boolean { | ||
| return oldItem == newItem | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. as the name suggest "areContents the same". This basically means if all the properties/members in the class are the same. So ideally you have to check if all the contents are the same... "oldItem == newItem" only checks if the object reference are the same |
||
| } | ||
|
|
||
|
|
@@ -40,7 +39,7 @@ class MovieListAdapter : ListAdapter<Content, MovieListAdapter.MovieViewHolder>( | |
| inner class MovieViewHolder(private val viewBinder: MovieItemBinding) : | ||
| RecyclerView.ViewHolder(viewBinder.root) { | ||
|
|
||
| fun viewBinding(data: Content) { | ||
| fun viewBinding(data: com.dhruvdroid.data.Content) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. file level import |
||
| viewBinder.title.text = data.name | ||
| viewBinder.movieCard.setBackgroundResource(getDrawableResource(data.posterImage)) | ||
| } | ||
|
|
||
This file was deleted.
This file was deleted.
This file was deleted.
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,6 @@ package com.dhruvdroid.sampleott.database | |
| import androidx.room.Dao | ||
| import androidx.room.Insert | ||
| import androidx.room.OnConflictStrategy | ||
| import com.dhruvdroid.sampleott.data.Tray | ||
|
|
||
| // | ||
| // Created by Dhruv on 14/08/20. | ||
|
|
@@ -13,7 +12,7 @@ import com.dhruvdroid.sampleott.data.Tray | |
| interface TrayDao { | ||
|
|
||
| @Insert(onConflict = OnConflictStrategy.REPLACE) | ||
| suspend fun insertMovieList(movieList: List<Tray>) | ||
| suspend fun insertMovieList(movieList: List<com.dhruvdroid.data.Tray>) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. file level import |
||
|
|
||
| // @Query("SELECT * FROM Tray WHERE page = :page_") | ||
| // suspend fun getMovieList(page_: Int): List<Tray> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,5 @@ | ||
| package com.dhruvdroid.sampleott.network | ||
|
|
||
| import com.dhruvdroid.sampleott.data.Tray | ||
| import retrofit2.http.GET | ||
| import retrofit2.http.Url | ||
|
|
||
|
|
@@ -11,5 +10,5 @@ import retrofit2.http.Url | |
| interface MovieService { | ||
|
|
||
| @GET | ||
| suspend fun fetchList(@Url path: String): Tray | ||
| suspend fun fetchList(@Url path: String): com.dhruvdroid.data.Tray | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. file level import |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,6 @@ | ||
| package com.dhruvdroid.sampleott.repository | ||
|
|
||
| import android.util.Log | ||
| import com.dhruvdroid.sampleott.data.Content | ||
| import com.dhruvdroid.sampleott.data.MovieResult | ||
| import com.dhruvdroid.sampleott.data.Tray | ||
| import com.dhruvdroid.sampleott.network.MovieService | ||
| import com.dhruvdroid.sampleott.utilities.AppUtils | ||
| import com.dhruvdroid.sampleott.utilities.AppUtils.getRandomId | ||
|
|
@@ -23,17 +20,17 @@ class MainRepository constructor( | |
| private val service: MovieService | ||
| ) : Repository { | ||
|
|
||
| private val searchResults = ConflatedBroadcastChannel<MovieResult>() | ||
| private val searchResults = ConflatedBroadcastChannel<com.dhruvdroid.data.MovieResult>() | ||
|
|
||
| // keep the last requested page. When the request is successful, increment the page number. | ||
| private var lastRequestedPage = START_PAGE_INDEX | ||
|
|
||
| // avoid triggering multiple requests in the same time | ||
| private var isApiInProgress = false | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lastRequestedPage and isApiInProgress are not suppose to be part of this file... The caller class should handle this |
||
|
|
||
| private var memoryCache = mutableListOf<Content>() | ||
| private var memoryCache = mutableListOf<com.dhruvdroid.data.Content>() | ||
|
|
||
| suspend fun getMovieList(): Flow<MovieResult> { | ||
| suspend fun getMovieList(): Flow<com.dhruvdroid.data.MovieResult> { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. file level import |
||
| lastRequestedPage = 1 | ||
| memoryCache.clear() | ||
| requestData() | ||
|
|
@@ -48,18 +45,18 @@ class MainRepository constructor( | |
| Log.d("--", "lastRequestedPage == $lastRequestedPage") | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove logs.... else create a separate file for logging that is injectable and call functions in it.. add a variable to disable log as per build type. |
||
| val response = service.fetchList(AppUtils.getApiUrl(lastRequestedPage)) | ||
| memoryCache.addAll(response.page.contentItems.content) | ||
| searchResults.offer(MovieResult.Success(filterItem(response))) | ||
| searchResults.offer(com.dhruvdroid.data.MovieResult.Success(filterItem(response))) | ||
| isSuccess = true | ||
| } catch (exception: IOException) { | ||
| searchResults.offer(MovieResult.Error(exception)) | ||
| searchResults.offer(com.dhruvdroid.data.MovieResult.Error(exception)) | ||
| } catch (exception: HttpException) { | ||
| searchResults.offer(MovieResult.Error(exception)) | ||
| searchResults.offer(com.dhruvdroid.data.MovieResult.Error(exception)) | ||
| } | ||
| isApiInProgress = false | ||
| return isSuccess | ||
| } | ||
|
|
||
| private fun filterItem(response: Tray): Tray { | ||
| private fun filterItem(response: com.dhruvdroid.data.Tray): com.dhruvdroid.data.Tray { | ||
| // adding random id to the API response | ||
| val array = ByteArray(7) | ||
| Random().nextBytes(array) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,10 +15,6 @@ import com.dhruvdroid.sampleott.R | |
| import com.dhruvdroid.sampleott.adapter.ContentListAdapter | ||
| import com.dhruvdroid.sampleott.adapter.MovieListAdapter | ||
| import com.dhruvdroid.sampleott.base.BaseActivity | ||
| import com.dhruvdroid.sampleott.data.Content | ||
| import com.dhruvdroid.sampleott.data.MovieResult | ||
| import com.dhruvdroid.sampleott.data.Page | ||
| import com.dhruvdroid.sampleott.data.Tray | ||
| import com.dhruvdroid.sampleott.layoutmanager.CustomGridLayoutManager | ||
| import com.dhruvdroid.sampleott.layoutmanager.ListDecorator | ||
| import com.dhruvdroid.sampleott.utilities.AppUtils | ||
|
|
@@ -35,7 +31,7 @@ class MainActivity : BaseActivity() { | |
|
|
||
| private var totalCount = 0 | ||
| private var contentAdapter: ContentListAdapter? = null | ||
| private lateinit var list: List<Content> | ||
| private lateinit var list: List<com.dhruvdroid.data.Content> | ||
| private val movieAdapter = MovieListAdapter() | ||
| private val viewModel: MainViewModel by viewModel() | ||
|
|
||
|
|
@@ -53,12 +49,12 @@ class MainActivity : BaseActivity() { | |
|
|
||
| viewModel.movieResult.observe(this) { result -> | ||
| when (result) { | ||
| is MovieResult.Success -> { | ||
| is com.dhruvdroid.data.MovieResult.Success -> { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Handle this in viewModel... and create two separate LiveData. One for success and other of error. |
||
| showEmptyList(result.data.page) | ||
| setUiData(result.data) | ||
| } | ||
|
|
||
| is MovieResult.Error -> { | ||
| is com.dhruvdroid.data.MovieResult.Error -> { | ||
| Toast.makeText( | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. write a reusable short hand function for Toast... maybe an extension function |
||
| this, | ||
| " $result.message}", | ||
|
|
@@ -69,13 +65,13 @@ class MainActivity : BaseActivity() { | |
| } | ||
| } | ||
|
|
||
| private fun setUiData(data: Tray) { | ||
| private fun setUiData(data: com.dhruvdroid.data.Tray) { | ||
| // list = data.page.contentItems.content | ||
| // movieAdapter.submitList(list) | ||
|
|
||
| if (contentAdapter == null) { | ||
| contentAdapter = | ||
| ContentListAdapter(data.page.contentItems.content as MutableList<Content>) | ||
| ContentListAdapter(data.page.contentItems.content as MutableList<com.dhruvdroid.data.Content>) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this casting done? we can use toMutableList(). |
||
| rvList.apply { | ||
| // use this setting to improve performance if you know that changes | ||
| // in content do not change the layout size of the RecyclerView | ||
|
|
@@ -91,13 +87,13 @@ class MainActivity : BaseActivity() { | |
| } | ||
| } else { | ||
| totalCount += data.page.contentItems.content.size | ||
| contentAdapter?.updateList(data.page.contentItems.content as MutableList<Content>) | ||
| contentAdapter?.updateList(data.page.contentItems.content as MutableList<com.dhruvdroid.data.Content>) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can be replaced to toMutableList() |
||
| } | ||
|
|
||
| setupScrollListener() | ||
| } | ||
|
|
||
| private fun showEmptyList(page: Page) { | ||
| private fun showEmptyList(page: com.dhruvdroid.data.Page) { | ||
| if (page.contentItems.content.isEmpty()) { | ||
| emptyList.visibility = View.VISIBLE | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. make reusable extension function. eg. makeVisible(), makeInvisible(), makeGone(). |
||
| rvList.visibility = View.GONE | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| /build |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| # Different types of JSON parsing technique | ||
|
|
||
| **Below mentioned - ** | ||
| 1. Moshi- | ||
| @Json(name = "vote_count") val voteCount: Int = -1 | ||
|
|
||
| 2. GSON- | ||
| @SerializedName(“vote_count”) val voteCount: Int = -1 | ||
|
|
||
| 3. Jackson - | ||
| @JsonProperty("vote_count") val voteCount: Int = -1 |
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.
Replace with RecyclerView.Adapter with RecyclerVIew.ListAdapter
https://medium.com/simform-engineering/listadapter-a-recyclerview-adapter-extension-5359d13bd879