Skip to content

Commit

Permalink
refactor: extract automatic answer Handler
Browse files Browse the repository at this point in the history
Removes mTimeoutHandler from AbstractFlashcardViewer

This is an incremental (bad) refactoring

Now we've removed the handler, we can move most of the internal
logic into the AutomaticAnswerSettings class

This will let us make more methods private
  • Loading branch information
david-allison authored and mikehardy committed Oct 13, 2021
1 parent 8e86539 commit 0ffc8d7
Show file tree
Hide file tree
Showing 3 changed files with 129 additions and 43 deletions.
61 changes: 24 additions & 37 deletions AnkiDroid/src/main/java/com/ichi2/anki/AbstractFlashcardViewer.java
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@
import static com.ichi2.anim.ActivityTransitionAnimation.Direction.*;

@SuppressWarnings({"PMD.AvoidThrowingRawExceptionTypes","PMD.FieldDeclarationsShouldBeAtStartOfClass"})
public abstract class AbstractFlashcardViewer extends NavigationDrawerActivity implements ReviewerUi, CommandProcessor, TagsDialogListener, WhiteboardMultiTouchMethods {
public abstract class AbstractFlashcardViewer extends NavigationDrawerActivity implements ReviewerUi, CommandProcessor, TagsDialogListener, WhiteboardMultiTouchMethods, AutomaticAnswerSettings.AutomaticallyAnswered {

/**
* Result codes that are returned when this activity finishes.
Expand Down Expand Up @@ -243,7 +243,7 @@ public abstract class AbstractFlashcardViewer extends NavigationDrawerActivity i
protected boolean mSpeakText;
protected boolean mDisableClipboard = false;

@NonNull protected AutomaticAnswerSettings mAutomaticAnswerSettings = new AutomaticAnswerSettings();
@NonNull protected AutomaticAnswerSettings mAutomaticAnswerSettings = AutomaticAnswerSettings.defaultInstance(this);

protected TypeAnswer mTypeAnswer;

Expand Down Expand Up @@ -401,7 +401,7 @@ public void onClick(View view) {
return;
}
mLastClickTime = getElapsedRealTime();
mTimeoutHandler.removeCallbacks(mShowAnswerTask);
mAutomaticAnswerSettings.stopShowingAnswer();
displayCardAnswer();
}
};
Expand Down Expand Up @@ -454,7 +454,7 @@ public void onClick(View view) {
view.setPressed(true);
}
mLastClickTime = getElapsedRealTime();
mTimeoutHandler.removeCallbacks(mShowQuestionTask);
mAutomaticAnswerSettings.stopShowingQuestion();
int id = view.getId();
if (id == R.id.flashcard_layout_ease1) {
Timber.i("AbstractFlashcardViewer:: EASE_1 pressed");
Expand Down Expand Up @@ -851,8 +851,7 @@ protected void onPause() {
super.onPause();
Timber.d("onPause()");

mTimeoutHandler.removeCallbacks(mShowAnswerTask);
mTimeoutHandler.removeCallbacks(mShowQuestionTask);
mAutomaticAnswerSettings.stopAll();
mLongClickHandler.removeCallbacks(mLongClickTestRunnable);
mLongClickHandler.removeCallbacks(mStartLongClickAction);

Expand Down Expand Up @@ -1732,7 +1731,7 @@ protected void restoreCollectionPreferences(Collection col) {
try {
mShowNextReviewTime = col.get_config_boolean("estTimes");
SharedPreferences preferences = AnkiDroidApp.getSharedPrefs(getBaseContext());
mAutomaticAnswerSettings = AutomaticAnswerSettings.createInstance(preferences, col);
mAutomaticAnswerSettings = AutomaticAnswerSettings.createInstance(this, preferences, col);
} catch (Exception ex) {
Timber.w(ex);
onCollectionLoadError();
Expand Down Expand Up @@ -1792,31 +1791,20 @@ protected void updateDeckName() {
}
}

/*
* Handler for the delay in auto showing question and/or answer One toggle for both question and answer, could set
* longer delay for auto next question
*/
@SuppressWarnings("deprecation") // #7111: new Handler()
protected final Handler mTimeoutHandler = new Handler();

protected final Runnable mShowQuestionTask = new Runnable() {
@Override
public void run() {
// Assume hitting the "Again" button when auto next question
if (mEase1Layout.isEnabled() && mEase1Layout.getVisibility() == View.VISIBLE) {
mEase1Layout.performClick();
}
@Override
public void automaticShowQuestion() {
// Assume hitting the "Again" button when auto next question
if (mEase1Layout.isEnabled() && mEase1Layout.getVisibility() == View.VISIBLE) {
mEase1Layout.performClick();
}
};
}

protected final Runnable mShowAnswerTask = new Runnable() {
@Override
public void run() {
if (mFlipCardLayout.isEnabled() && mFlipCardLayout.getVisibility() == View.VISIBLE) {
mFlipCardLayout.performClick();
}
@Override
public void automaticShowAnswer() {
if (mFlipCardLayout.isEnabled() && mFlipCardLayout.getVisibility() == View.VISIBLE) {
mFlipCardLayout.performClick();
}
};
}

class ReadTextListener implements ReadText.ReadTextListener {
public void onDone() {
Expand All @@ -1826,12 +1814,12 @@ public void onDone() {
if (ReadText.getmQuestionAnswer() == SoundSide.QUESTION) {
long delay = mAutomaticAnswerSettings.getAnswerDelayMilliseconds();
if (delay > 0) {
mTimeoutHandler.postDelayed(mShowAnswerTask, delay);
mAutomaticAnswerSettings.delayedShowAnswer(delay);
}
} else if (ReadText.getmQuestionAnswer() == SoundSide.ANSWER) {
long delay = mAutomaticAnswerSettings.getQuestionDelayMilliseconds();
if (delay > 0) {
mTimeoutHandler.postDelayed(mShowQuestionTask, delay);
mAutomaticAnswerSettings.delayedShowQuestion(delay);
}
}
}
Expand Down Expand Up @@ -1914,10 +1902,10 @@ protected void displayCardQuestion(boolean reload) {
// If the user wants to show the answer automatically
if (mAutomaticAnswerSettings.useTimer()) {
if (mAutomaticAnswerSettings.autoAdvanceAnswer()) {
mTimeoutHandler.removeCallbacks(mShowAnswerTask);
mAutomaticAnswerSettings.stopShowingAnswer();
if (!mSpeakText) {
long delay = mAutomaticAnswerSettings.getAnswerDelayMilliseconds() + mUseTimerDynamicMS;
mTimeoutHandler.postDelayed(mShowAnswerTask, delay);
mAutomaticAnswerSettings.delayedShowAnswer(delay);
}
}
}
Expand Down Expand Up @@ -1964,10 +1952,10 @@ protected void displayCardAnswer() {
// If the user wants to show the next question automatically
if (mAutomaticAnswerSettings.useTimer()) {
if (mAutomaticAnswerSettings.autoAdvanceQuestion()) {
mTimeoutHandler.removeCallbacks(mShowQuestionTask);
mAutomaticAnswerSettings.stopShowingQuestion();
if (!mSpeakText) {
long delay = mAutomaticAnswerSettings.getQuestionDelayMilliseconds() + mUseTimerDynamicMS;
mTimeoutHandler.postDelayed(mShowQuestionTask, delay);
mAutomaticAnswerSettings.delayedShowQuestion(delay);
}
}
}
Expand Down Expand Up @@ -2678,8 +2666,7 @@ protected void closeReviewer(int result, boolean saveDeck) {
}
}

mTimeoutHandler.removeCallbacks(mShowAnswerTask);
mTimeoutHandler.removeCallbacks(mShowQuestionTask);
mAutomaticAnswerSettings.stopAll();
mTimerHandler.removeCallbacks(mRemoveChosenAnswerText);
mLongClickHandler.removeCallbacks(mLongClickTestRunnable);
mLongClickHandler.removeCallbacks(mStartLongClickAction);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,12 @@
package com.ichi2.anki.reviewer

import android.content.SharedPreferences
import android.os.Handler
import com.ichi2.libanki.Collection

// TODO: Settings should not be aware of the target
class AutomaticAnswerSettings(
target: AutomaticallyAnswered,
@get:JvmName("useTimer") val useTimer: Boolean = false,
private val questionDelaySeconds: Int = 60,
private val answerDelaySeconds: Int = 20
Expand All @@ -35,9 +38,49 @@ class AutomaticAnswerSettings(
@get:JvmName("autoAdvanceQuestion")
val autoAdvanceQuestion; get() = questionDelaySeconds > 0

private val showAnswerTask = Runnable(target::automaticShowAnswer)
private val showQuestionTask = Runnable(target::automaticShowQuestion)

/**
* Handler for the delay in auto showing question and/or answer
* One toggle for both question and answer, could set longer delay for auto next question
*/
@Suppress("Deprecation") // #7111: new Handler()
val timeoutHandler = Handler()

fun delayedShowQuestion(delay: Long) {
timeoutHandler.postDelayed(showQuestionTask, delay)
}

fun delayedShowAnswer(delay: Long) {
timeoutHandler.postDelayed(showAnswerTask, delay)
}

fun stopShowingQuestion() {
timeoutHandler.removeCallbacks(showQuestionTask)
}

fun stopShowingAnswer() {
timeoutHandler.removeCallbacks(showAnswerTask)
}

fun stopAll() {
stopShowingAnswer()
stopShowingQuestion()
}

interface AutomaticallyAnswered {
fun automaticShowAnswer()
fun automaticShowQuestion()
}

companion object {
@JvmStatic
fun queryDeckSpecificOptions(col: Collection, selectedDid: Long): AutomaticAnswerSettings? {
fun queryDeckSpecificOptions(
target: AutomaticallyAnswered,
col: Collection,
selectedDid: Long
): AutomaticAnswerSettings? {
// Dynamic don't have review options; attempt to get deck-specific auto-advance options
// but be prepared to go with all default if it's a dynamic deck
if (col.decks.isDyn(selectedDid)) {
Expand All @@ -54,21 +97,26 @@ class AutomaticAnswerSettings(
val useTimer = revOptions.optBoolean("timeoutAnswer", false)
val waitQuestionSecond = revOptions.optInt("timeoutQuestionSeconds", 60)
val waitAnswerSecond = revOptions.optInt("timeoutAnswerSeconds", 20)
return AutomaticAnswerSettings(useTimer, waitQuestionSecond, waitAnswerSecond)
return AutomaticAnswerSettings(target, useTimer, waitQuestionSecond, waitAnswerSecond)
}

@JvmStatic
fun queryFromPreferences(preferences: SharedPreferences): AutomaticAnswerSettings {
fun queryFromPreferences(target: AutomaticallyAnswered, preferences: SharedPreferences): AutomaticAnswerSettings {
val prefUseTimer: Boolean = preferences.getBoolean("timeoutAnswer", false)
val prefWaitQuestionSecond: Int = preferences.getInt("timeoutQuestionSeconds", 60)
val prefWaitAnswerSecond: Int = preferences.getInt("timeoutAnswerSeconds", 20)
return AutomaticAnswerSettings(prefUseTimer, prefWaitQuestionSecond, prefWaitAnswerSecond)
return AutomaticAnswerSettings(target, prefUseTimer, prefWaitQuestionSecond, prefWaitAnswerSecond)
}

@JvmStatic
fun createInstance(preferences: SharedPreferences, col: Collection): AutomaticAnswerSettings {
fun createInstance(target: AutomaticallyAnswered, preferences: SharedPreferences, col: Collection): AutomaticAnswerSettings {
// deck specific options take precedence over general (preference-based) options
return queryDeckSpecificOptions(col, col.decks.selected()) ?: queryFromPreferences(preferences)
return queryDeckSpecificOptions(target, col, col.decks.selected()) ?: queryFromPreferences(target, preferences)
}

@JvmStatic
fun defaultInstance(target: AutomaticallyAnswered): AutomaticAnswerSettings {
return AutomaticAnswerSettings(target)
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/*
* Copyright (c) 2021 David Allison <[email protected]>
*
* This program is free software; you can redistribute it and/or modify it under
* the terms of the GNU General Public License as published by the Free Software
* Foundation; either version 3 of the License, or (at your option) any later
* version.
*
* This program is distributed in the hope that it will be useful, but WITHOUT ANY
* WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A
* PARTICULAR PURPOSE. See the GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License along with
* this program. If not, see <http://www.gnu.org/licenses/>.
*/

package com.ichi2.anki.reviewer

import androidx.test.ext.junit.runners.AndroidJUnit4
import com.ichi2.testutils.EmptyApplication
import org.hamcrest.CoreMatchers.equalTo
import org.hamcrest.MatcherAssert.assertThat
import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.kotlin.mock
import org.robolectric.annotation.Config

@RunWith(AndroidJUnit4::class)
@Config(application = EmptyApplication::class)
class AutomaticAnswerSettingsTest {

@Test
fun stopAllWorks() {
val target: AutomaticAnswerSettings.AutomaticallyAnswered = mock()
val answer = AutomaticAnswerSettings(
useTimer = true,
questionDelaySeconds = 10,
answerDelaySeconds = 10,
target = target
)

answer.delayedShowQuestion(0)
answer.delayedShowAnswer(0)

assertThat(answer.timeoutHandler.hasMessages(0), equalTo(true))

answer.stopAll()

assertThat(answer.timeoutHandler.hasMessages(0), equalTo(false))
}
}

0 comments on commit 0ffc8d7

Please sign in to comment.