Skip to content

Commit

Permalink
refactor: cleanups
Browse files Browse the repository at this point in the history
refactor: toSentenceCase

1. Replace Activity with Context (activities are also Context)
2. Add method that accepts `Resources`
3. Remove repeated code

fix: check if HeaderFragment is attached

parentFragmentManager may throw if not

refactor: inline ViewerCommand::allDefaultBindings

Only used in tests

refactor: use Kotlin notation instead of java BiFunction

refactor: inline removeBinding

only used in a test

refactor: inline keyMap

IDE suggestion
  • Loading branch information
BrayanDSO committed Dec 10, 2024
1 parent 66c5403 commit 0092e2f
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 53 deletions.
52 changes: 15 additions & 37 deletions AnkiDroid/src/main/java/com/ichi2/anki/cardviewer/ViewerCommand.kt
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,6 @@ import com.ichi2.anki.reviewer.MappableBinding
import com.ichi2.anki.reviewer.MappableBinding.Companion.fromPreference
import com.ichi2.anki.reviewer.MappableBinding.Companion.toPreferenceString
import com.ichi2.anki.reviewer.MappableBinding.Screen
import java.util.Arrays
import java.util.function.BiFunction
import java.util.stream.Collectors

/** Abstraction: Discuss moving many of these to 'Reviewer' */
enum class ViewerCommand(
Expand Down Expand Up @@ -91,13 +88,6 @@ enum class ViewerCommand(
;

companion object {
val allDefaultBindings: List<MappableBinding>
get() =
Arrays
.stream(entries.toTypedArray())
.flatMap { x: ViewerCommand -> x.defaultValue.stream() }
.collect(Collectors.toList())

fun fromPreferenceKey(key: String) = entries.first { it.preferenceKey == key }
}

Expand All @@ -108,54 +98,42 @@ enum class ViewerCommand(
preferences: SharedPreferences,
binding: MappableBinding,
) {
val addAtStart =
BiFunction { collection: MutableList<MappableBinding>, element: MappableBinding ->
// reorder the elements, moving the added binding to the first position
collection.remove(element)
collection.add(0, element)
true
}
val addAtStart: (MutableList<MappableBinding>, MappableBinding) -> Boolean = { collection, element ->
// reorder the elements, moving the added binding to the first position
collection.remove(element)
collection.add(0, element)
true
}
addBindingInternal(preferences, binding, addAtStart)
}

fun addBindingAtEnd(
preferences: SharedPreferences,
binding: MappableBinding,
) {
val addAtEnd =
BiFunction { collection: MutableList<MappableBinding>, element: MappableBinding ->
// do not reorder the elements
if (collection.contains(element)) {
return@BiFunction false
}
val addAtEnd: (MutableList<MappableBinding>, MappableBinding) -> Boolean = { collection, element ->
// do not reorder the elements
if (collection.contains(element)) {
false
} else {
collection.add(element)
return@BiFunction true
true
}
}
addBindingInternal(preferences, binding, addAtEnd)
}

private fun addBindingInternal(
preferences: SharedPreferences,
binding: MappableBinding,
performAdd: BiFunction<MutableList<MappableBinding>, MappableBinding, Boolean>,
performAdd: (MutableList<MappableBinding>, MappableBinding) -> Boolean,
) {
val bindings: MutableList<MappableBinding> = fromPreference(preferences, this)
performAdd.apply(bindings, binding)
performAdd(bindings, binding)
val newValue: String = bindings.toPreferenceString()
preferences.edit { putString(preferenceKey, newValue) }
}

fun removeBinding(
prefs: SharedPreferences,
binding: MappableBinding,
) {
val bindings: MutableList<MappableBinding> = MappableBinding.fromPreferenceString(preferenceKey)
bindings.remove(binding)
prefs.edit {
putString(preferenceKey, bindings.toPreferenceString())
}
}

// If we use the serialised format, then this adds additional coupling to the properties.
val defaultValue: List<MappableBinding>
get() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ class HeaderFragment :
}

parentFragmentManager.addOnBackStackChangedListener {
if (!isAdded) return@addOnBackStackChangedListener
val fragment =
parentFragmentManager.findFragmentById(R.id.settings_container)
?: return@addOnBackStackChangedListener
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class PeripheralKeymap(
reviewerUi: ReviewerUi,
commandProcessor: ViewerCommand.CommandProcessor,
) {
private val keyMap: KeyMap
private val keyMap: KeyMap = KeyMap(commandProcessor, reviewerUi) { Screen.Reviewer(it) }
private var hasSetup = false

fun setup() {
Expand Down Expand Up @@ -110,8 +110,4 @@ class PeripheralKeymap(

operator fun get(key: MappableBinding): ViewerCommand? = bindingMap[key]
}

init {
keyMap = KeyMap(commandProcessor, reviewerUi) { Screen.Reviewer(it) }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

package com.ichi2.anki.ui.internationalization

import android.app.Activity
import android.content.Context
import androidx.annotation.StringRes
import androidx.fragment.app.Fragment

Expand All @@ -30,10 +30,10 @@ import androidx.fragment.app.Fragment
* ```
*/
fun String.toSentenceCase(
activity: Activity,
context: Context,
@StringRes resId: Int,
): String {
val resString = activity.getString(resId)
val resString = context.getString(resId)
// lowercase both for the comparison: sentence case doesn't mean all words are lowercase
if (this.lowercase() == resString.lowercase()) return resString
return this
Expand All @@ -42,9 +42,4 @@ fun String.toSentenceCase(
fun String.toSentenceCase(
fragment: Fragment,
@StringRes resId: Int,
): String {
val resString = fragment.getString(resId)
// lowercase both for the comparison: sentence case doesn't mean all words are lowercase
if (this.lowercase() == resString.lowercase()) return resString
return this
}
): String = toSentenceCase(fragment.requireContext(), resId)
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import com.ichi2.anki.reviewer.Binding
import com.ichi2.anki.reviewer.FullScreenMode
import com.ichi2.anki.reviewer.FullScreenMode.Companion.setPreference
import com.ichi2.anki.reviewer.MappableBinding
import com.ichi2.anki.reviewer.MappableBinding.Companion.toPreferenceString
import com.ichi2.anki.reviewer.MappableBinding.Screen
import com.ichi2.libanki.Consts
import com.ichi2.libanki.DeckId
Expand Down Expand Up @@ -299,7 +300,12 @@ class ReviewerNoParamTest : RobolectricTest() {
for (mappableBinding in MappableBinding.fromPreference(prefs, command)) {
val gestureBinding = mappableBinding.binding as? Binding.GestureInput? ?: continue
if (gestureBinding.gesture in gestures) {
command.removeBinding(prefs, mappableBinding)
val bindings: MutableList<MappableBinding> =
MappableBinding.fromPreferenceString(command.preferenceKey)
bindings.remove(mappableBinding)
prefs.edit {
putString(command.preferenceKey, bindings.toPreferenceString())
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,13 @@ package com.ichi2.anki.reviewer

import android.view.KeyEvent
import com.ichi2.anki.cardviewer.ViewerCommand
import com.ichi2.anki.cardviewer.ViewerCommand.entries
import org.hamcrest.MatcherAssert.assertThat
import org.hamcrest.Matchers.hasItem
import org.hamcrest.Matchers.not
import org.junit.Test
import java.util.Arrays
import java.util.stream.Collectors

class MappableBindingTest {
@Test
Expand All @@ -39,7 +42,11 @@ class MappableBindingTest {
assertThat(allBindings, not(hasItem(keyCode(KeyEvent.KEYCODE_L))))
}

private fun getAllBindings() = ViewerCommand.allDefaultBindings
private fun getAllBindings() =
Arrays
.stream(entries.toTypedArray())
.flatMap { x: ViewerCommand -> x.defaultValue.stream() }
.collect(Collectors.toList())

@Suppress("SameParameterValue")
private fun keyCode(code: Int) = fromBinding(BindingTest.keyCode(code))
Expand Down

0 comments on commit 0092e2f

Please sign in to comment.