Skip to content
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

Feature/issue550 add equalable content provider #766

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import androidx.navigation.NavDirections
import com.squareup.inject.assisted.Assisted
import com.squareup.inject.assisted.AssistedInject
import com.xwray.groupie.databinding.BindableItem
import io.github.droidkaigi.confsched2019.item.EqualableContentsProvider
import io.github.droidkaigi.confsched2019.model.ServiceSession
import io.github.droidkaigi.confsched2019.model.defaultLang
import io.github.droidkaigi.confsched2019.session.R
Expand All @@ -19,7 +20,7 @@ class ServiceSessionItem @AssistedInject constructor(
val sessionContentsActionCreator: SessionContentsActionCreator
) : BindableItem<ItemServiceSessionBinding>(
session.id.hashCode().toLong()
), SessionItem {
), SessionItem, EqualableContentsProvider {
val serviceSession get() = session

@AssistedInject.Factory
Expand Down Expand Up @@ -63,18 +64,13 @@ class ServiceSessionItem @AssistedInject constructor(

override fun getLayout(): Int = R.layout.item_service_session

override fun equals(other: Any?): Boolean {
if (this === other) return true
if (javaClass != other?.javaClass) return false

other as ServiceSessionItem
override fun providerEqualableContents(): Array<*> = arrayOf(session)

if (session != other.session) return false

return true
override fun equals(other: Any?): Boolean {
return isSameContents(other)
}

override fun hashCode(): Int {
return session.hashCode()
return contentsHash()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import com.squareup.inject.assisted.Assisted
import com.squareup.inject.assisted.AssistedInject
import com.squareup.picasso.Picasso
import com.xwray.groupie.databinding.BindableItem
import io.github.droidkaigi.confsched2019.item.EqualableContentsProvider
import io.github.droidkaigi.confsched2019.model.Speaker
import io.github.droidkaigi.confsched2019.session.R
import io.github.droidkaigi.confsched2019.session.databinding.ItemSpeakerBinding
Expand All @@ -18,7 +19,8 @@ class SpeakerItem @AssistedInject constructor(
@Assisted val clickNavDirection: NavDirections,
@Assisted val query: String?,
val navController: NavController
) : BindableItem<ItemSpeakerBinding>(speaker.id.hashCode().toLong()) {
) : BindableItem<ItemSpeakerBinding>(speaker.id.hashCode().toLong()),
EqualableContentsProvider {
@AssistedInject.Factory
interface Factory {
fun create(
Expand Down Expand Up @@ -64,19 +66,17 @@ class SpeakerItem @AssistedInject constructor(
}
}

override fun equals(other: Any?): Boolean {
if (this === other) return true
if (javaClass != other?.javaClass) return false

other as SpeakerItem
override fun providerEqualableContents(): Array<*> = arrayOf(speaker)

if (speaker != other.speaker) return false
if (query != other.query) return false
override fun isTextHighlightNeedUpdate() = query?.let {
Copy link
Member

Choose a reason for hiding this comment

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

ちょっと日本語で失礼します。 🙇
isTextHighlightNeedUpdateだと問題があるかもしれません。なぜなら例えばandroidとか入れている時に、1つのセッションをfavoriteするとマッチしている他のセッションも全て点滅してしまいます。
個人的にはproviderEqualableContentsでヒットしているときのみqueryを入れるのがバグがなくて一番良いと思っていますがどうでしょうか?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

すみません、最初にissueでコメントして頂いた意図を汲みきれていなかったみたいですね。
providerEqualableContentsにqueryを含めることでequalにならない様にする、というところまで読み切れていませんでした。
確かにチェックする箇所を1箇所にまとめる方がバグの混入が防げて良さそうですね。
修正されたコードで軽く動作確認しましたが、入力や削除時の動作で特に気になる点はなかったので問題ないかと思います 🙇‍♂️

Copy link
Member

Choose a reason for hiding this comment

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

ありがとうございます! 🙇

speaker.name.toLowerCase().contains(it.toLowerCase())
} ?: false

return true
override fun equals(other: Any?): Boolean {
return isSameContents(other)
}

override fun hashCode(): Int {
return speaker.hashCode() + query.hashCode()
return contentsHash()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import com.squareup.inject.assisted.AssistedInject
import com.squareup.picasso.Picasso
import com.squareup.picasso.Target
import com.xwray.groupie.databinding.BindableItem
import io.github.droidkaigi.confsched2019.item.EqualableContentsProvider
import io.github.droidkaigi.confsched2019.model.Speaker
import io.github.droidkaigi.confsched2019.model.SpeechSession
import io.github.droidkaigi.confsched2019.model.defaultLang
Expand All @@ -40,7 +41,7 @@ class SpeechSessionItem @AssistedInject constructor(
val sessionContentsActionCreator: SessionContentsActionCreator
) : BindableItem<ItemSessionBinding>(
session.id.hashCode().toLong()
), SessionItem {
), SessionItem, EqualableContentsProvider {
val speechSession get() = session

@AssistedInject.Factory
Expand Down Expand Up @@ -199,19 +200,24 @@ class SpeechSessionItem @AssistedInject constructor(

override fun getLayout(): Int = R.layout.item_session

override fun equals(other: Any?): Boolean {
if (this === other) return true
if (javaClass != other?.javaClass) return false

other as SpeechSessionItem
override fun providerEqualableContents(): Array<*> = arrayOf(session)

if (session != other.session) return false
if (query == null || other.query == null || query != other.query) return false
override fun isTextHighlightNeedUpdate() = query?.let {
when {
session.title.getByLang(defaultLang()).toLowerCase()
.contains(query.toLowerCase()) -> true
session.speakers.find {
it.name.toLowerCase().contains(query.toLowerCase())
} != null -> true
else -> false
}
} ?: false

return true
override fun equals(other: Any?): Boolean {
return isSameContents(other)
}

override fun hashCode(): Int {
return session.hashCode() + query.hashCode()
return contentsHash()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import androidx.navigation.NavDirections
import com.squareup.inject.assisted.Assisted
import com.squareup.inject.assisted.AssistedInject
import com.xwray.groupie.databinding.BindableItem
import io.github.droidkaigi.confsched2019.item.EqualableContentsProvider
import io.github.droidkaigi.confsched2019.model.ServiceSession
import io.github.droidkaigi.confsched2019.model.defaultLang
import io.github.droidkaigi.confsched2019.session.R
Expand All @@ -17,7 +18,7 @@ class TabularServiceSessionItem @AssistedInject constructor(
private val navController: NavController
) : BindableItem<ItemTabularServiceSessionBinding>(
session.id.hashCode().toLong()
), SessionItem {
), SessionItem, EqualableContentsProvider {

@AssistedInject.Factory
interface Factory {
Expand Down Expand Up @@ -46,18 +47,13 @@ class TabularServiceSessionItem @AssistedInject constructor(

override fun getLayout() = R.layout.item_tabular_service_session

override fun equals(other: Any?): Boolean {
if (this === other) return true
if (javaClass != other?.javaClass) return false

other as TabularServiceSessionItem
override fun providerEqualableContents(): Array<*> = arrayOf(session)

if (session != other.session) return false

return true
override fun equals(other: Any?): Boolean {
return isSameContents(other)
}

override fun hashCode(): Int {
return session.hashCode()
return contentsHash()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import com.squareup.picasso.Picasso
import com.xwray.groupie.GroupAdapter
import com.xwray.groupie.databinding.BindableItem
import com.xwray.groupie.databinding.ViewHolder
import io.github.droidkaigi.confsched2019.item.EqualableContentsProvider
import io.github.droidkaigi.confsched2019.model.Speaker
import io.github.droidkaigi.confsched2019.model.SpeechSession
import io.github.droidkaigi.confsched2019.model.defaultLang
Expand All @@ -26,7 +27,7 @@ class TabularSpeechSessionItem @AssistedInject constructor(
private val navController: NavController
) : BindableItem<ItemTabularSpeechSessionBinding>(
session.id.hashCode().toLong()
), SessionItem {
), SessionItem, EqualableContentsProvider {

@AssistedInject.Factory
interface Factory {
Expand Down Expand Up @@ -82,19 +83,14 @@ class TabularSpeechSessionItem @AssistedInject constructor(

override fun getLayout() = R.layout.item_tabular_speech_session

override fun equals(other: Any?): Boolean {
if (this === other) return true
if (javaClass != other?.javaClass) return false

other as TabularSpeechSessionItem
override fun providerEqualableContents(): Array<*> = arrayOf(session)

if (session != other.session) return false

return true
override fun equals(other: Any?): Boolean {
return isSameContents(other)
}

override fun hashCode(): Int {
return session.hashCode()
return contentsHash()
}

private class SpeakerIconItem(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package io.github.droidkaigi.confsched2019.item

import java.util.Arrays

interface EqualableContentsProvider {
fun providerEqualableContents(): Array<*>

override fun equals(other: Any?): Boolean

override fun hashCode(): Int

fun isTextHighlightNeedUpdate(): Boolean {
return false
}

fun isSameContents(other: Any?): Boolean {
other ?: return false
if (other !is EqualableContentsProvider) return false
if (other::class != this::class) return false
if (isTextHighlightNeedUpdate()) return false
if (other.isTextHighlightNeedUpdate()) return false
return other.providerEqualableContents().contentDeepEquals(this.providerEqualableContents())
}

fun contentsHash(): Int {
return Arrays.deepHashCode(arrayOf(this::class, this.providerEqualableContents()))
}
}