Skip to content

Refactoring arch#1

Open
dhruvdroid wants to merge 2 commits intomasterfrom
refactoring_arch
Open

Refactoring arch#1
dhruvdroid wants to merge 2 commits intomasterfrom
refactoring_arch

Conversation

@dhruvdroid
Copy link
Owner

No description provided.

RecyclerView.ViewHolder(viewBinder.root) {

fun viewBinding(data: Content) {
fun viewBinding(data: com.dhruvdroid.data.Content) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add in file level import

}

fun updateList(content: List<Content>) {
fun updateList(content: List<com.dhruvdroid.data.Content>) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add in file level import

@@ -15,7 +15,7 @@ import com.dhruvdroid.sampleott.databinding.MovieItemBinding
class ContentListAdapter(val list: MutableList<Content>) :
RecyclerView.Adapter<ContentListAdapter.MovieViewHolder>(), Filterable {
Copy link
Collaborator

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

} else {
val filteredList: MutableList<Content> = ArrayList()
val filteredList: MutableList<com.dhruvdroid.data.Content> = ArrayList()
for (item in list) {
Copy link
Collaborator

Choose a reason for hiding this comment

The 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.
It's pointless to use Kotlin if we are still writing code in Java way.
val filteredList = list.filter { it.name.toLowerCase().contains(charString.toLowerCase()) }


override fun publishResults(charSequence: CharSequence, filterResults: FilterResults) {
listFiltered = filterResults.values as MutableList<Content>
listFiltered = filterResults.values as MutableList<com.dhruvdroid.data.Content>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add file level import

private fun showEmptyList(page: Page) {
private fun showEmptyList(page: com.dhruvdroid.data.Page) {
if (page.contentItems.content.isEmpty()) {
emptyList.visibility = View.VISIBLE
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make reusable extension function. eg. makeVisible(), makeInvisible(), makeGone().

implementation "org.jetbrains.kotlin:kotlin-stdlib-jdk7:$kotlin_version"
implementation "org.jetbrains.kotlinx:kotlinx-serialization-runtime:0.20.0"
implementation "com.squareup.moshi:moshi-kotlin:1.9.3"
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that you are using two serialization libraries... opt for one. either Kotlinx or Moshi

dependencies {
implementation fileTree(dir: 'libs', include: ['*.jar'])
implementation "org.jetbrains.kotlin:kotlin-stdlib-jdk7:$kotlin_version"
implementation "org.jetbrains.kotlinx:kotlinx-serialization-runtime:0.20.0"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kotlinx-serialization-runtime is replaced with kotlinx.core. to use this update kotlin to 1.4.0
https://github.com/Kotlin/kotlinx.serialization

}

sourceCompatibility = "1.7"
targetCompatibility = "1.7" No newline at end of file
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this be 1.8 ?

@@ -0,0 +1,4 @@
package com.dhruvdroid.domain

class MyClass {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

delete empty class or add a TODO

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants