Restoring the instance state of an SQLite quiz app in Android

Question

The following class is the activity of a little SQLite quiz app, which takes care of showing the actual quiz. Other than that the app only has a starting screen. I want to save the instance state and restore it. It works, but I wonder if the code is too messy now. I am a beginner of course.

Where should I remove redundancy? How is this overall approach? I save texts of TextViews, the button and the RadioButtons with the freezesText=true xml attribute.

public class QuizActivity extends AppCompatActivity {
public static final String EXTRA_SCORE = "extraScore";
private static final long COUNTDOWN_IN_MILLIS = 30000;
private static final String KEY_SCORE = "keyScore";
private static final String KEY_QUESTION_COUNT = "keyQuestionCount";
private static final String KEY_MILLIS_LEFT = "keyMillisLeft";
private static final String KEY_QUESTION_LIST = "keyQuestionList";
private static final String KEY_ANSWERED = "keyAnswered";
private TextView textViewQuestion;
private TextView textViewScore;
private TextView textViewQuestionCount;
private TextView textViewCountDown;
private RadioGroup rbGroup;
private RadioButton rb1;
private RadioButton rb2;
private RadioButton rb3;
private Button buttonConfirmNext;
private ColorStateList textColorDefaultRb;
private ColorStateList textColorDefaultCd;
private CountDownTimer countDownTimer;
private long timeLeftInMillis;
private List<Question> questionList;
private int questionCounter;
private int questionCountTotal;
private Question currentQuestion;
private int score;
private boolean answered;
private long backPressedTime;
@Override
protected void onCreate(Bundle savedInstanceState) {
 super.onCreate(savedInstanceState);
 setContentView(R.layout.activity_quiz);
 textViewQuestion = findViewById(R.id.text_view_question);
 textViewScore = findViewById(R.id.text_view_score);
 textViewQuestionCount = findViewById(R.id.text_view_question_count);
 textViewCountDown = findViewById(R.id.text_view_countdown);
 rbGroup = findViewById(R.id.radio_group);
 rb1 = findViewById(R.id.radio_button1);
 rb2 = findViewById(R.id.radio_button2);
 rb3 = findViewById(R.id.radio_button3);
 buttonConfirmNext = findViewById(R.id.button_confirm_next);
 textColorDefaultRb = rb1.getTextColors();
 textColorDefaultCd = textViewCountDown.getTextColors();
 if (savedInstanceState == null) {
 QuizDbHelper dbHelper = new QuizDbHelper(this);
 questionList = dbHelper.getAllQuestions();
 questionCountTotal = questionList.size();
 Collections.shuffle(questionList);
 showNextQuestion();
 } else {
 questionList = savedInstanceState.getParcelableArrayList(KEY_QUESTION_LIST);
 questionCountTotal = questionList.size();
 questionCounter = savedInstanceState.getInt(KEY_QUESTION_COUNT);
 currentQuestion = questionList.get(questionCounter-1);
 score = savedInstanceState.getInt(KEY_SCORE);
 timeLeftInMillis = savedInstanceState.getLong(KEY_MILLIS_LEFT);
 answered = savedInstanceState.getBoolean(KEY_ANSWERED);
 if (!answered) {
 startCountDown();
 } else {
 showSolution();
 }
 }
 buttonConfirmNext.setOnClickListener(new View.OnClickListener() {
 @Override
 public void onClick(View v) {
 if (!answered) {
 if (rb1.isChecked() || rb2.isChecked() || rb3.isChecked()) {
 checkAnswer();
 } else {
 Toast.makeText(QuizActivity.this, "Please select an answer", Toast.LENGTH_SHORT).show();
 }
 } else {
 showNextQuestion();
 }
 }
 });
}
private void showNextQuestion() {
 rb1.setTextColor(textColorDefaultRb);
 rb2.setTextColor(textColorDefaultRb);
 rb3.setTextColor(textColorDefaultRb);
 rbGroup.clearCheck();
 if (questionCounter < questionCountTotal) {
 currentQuestion = questionList.get(questionCounter);
 textViewQuestion.setText(currentQuestion.getQuestion());
 rb1.setText(currentQuestion.getOption1());
 rb2.setText(currentQuestion.getOption2());
 rb3.setText(currentQuestion.getOption3());
 questionCounter++;
 textViewQuestionCount.setText("Question: " + questionCounter + "/" + questionCountTotal);
 answered = false;
 buttonConfirmNext.setText("Confirm");
 timeLeftInMillis = COUNTDOWN_IN_MILLIS;
 startCountDown();
 } else {
 finishQuiz();
 }
}
private void startCountDown() {
 countDownTimer = new CountDownTimer(timeLeftInMillis, 1000) {
 @Override
 public void onTick(long millisUntilFinished) {
 timeLeftInMillis = millisUntilFinished;
 updateCountDownText();
 }
 @Override
 public void onFinish() {
 timeLeftInMillis = 0;
 updateCountDownText();
 checkAnswer();
 }
 }.start();
}
private void updateCountDownText() {
 int minutes = (int) (timeLeftInMillis / 1000) / 60;
 int seconds = (int) (timeLeftInMillis / 1000) % 60;
 String timeFormatted = String.format(Locale.getDefault(), "%02d:%02d", minutes, seconds);
 textViewCountDown.setText(timeFormatted);
 if (timeLeftInMillis < 10000) {
 textViewCountDown.setTextColor(Color.RED);
 } else {
 textViewCountDown.setTextColor(textColorDefaultCd);
 }
}
private void checkAnswer() {
 answered = true;
 countDownTimer.cancel();
 RadioButton rbSelected = findViewById(rbGroup.getCheckedRadioButtonId());
 int answerNr = rbGroup.indexOfChild(rbSelected) + 1;
 if (answerNr == currentQuestion.getAnswerNr()) {
 score++;
 textViewScore.setText("Score: " + score);
 }
 showSolution();
}
private void showSolution() {
 rb1.setTextColor(Color.RED);
 rb2.setTextColor(Color.RED);
 rb3.setTextColor(Color.RED);
 switch (currentQuestion.getAnswerNr()) {
 case 1:
 rb1.setTextColor(Color.GREEN);
 textViewQuestion.setText("Answer 1 is correct");
 break;
 case 2:
 rb2.setTextColor(Color.GREEN);
 textViewQuestion.setText("Answer 2 is correct");
 break;
 case 3:
 rb3.setTextColor(Color.GREEN);
 textViewQuestion.setText("Answer 3 is correct");
 break;
 }
 if (questionCounter < questionCountTotal) {
 buttonConfirmNext.setText("Next");
 } else {
 buttonConfirmNext.setText("Finish");
 }
}
private void finishQuiz() {
 Intent resultIntent = new Intent();
 resultIntent.putExtra(EXTRA_SCORE, score);
 setResult(RESULT_OK, resultIntent);
 finish();
}
@Override
public void onBackPressed() {
 if (backPressedTime + 2000 > System.currentTimeMillis()) {
 finishQuiz();
 } else {
 Toast.makeText(this, "Press back again to finish", Toast.LENGTH_SHORT).show();
 }
 backPressedTime = System.currentTimeMillis();
}
@Override
protected void onDestroy() {
 super.onDestroy();
 if (countDownTimer != null) {
 countDownTimer.cancel();
 }
}
@Override
protected void onSaveInstanceState(Bundle outState) {
 super.onSaveInstanceState(outState);
 outState.putInt(KEY_SCORE, score);
 outState.putInt(KEY_QUESTION_COUNT, questionCounter);
 outState.putLong(KEY_MILLIS_LEFT, timeLeftInMillis);
 outState.putParcelableArrayList(KEY_QUESTION_LIST, (ArrayList<? extends Parcelable>) questionList);
 outState.putBoolean(KEY_ANSWERED, answered);
}
}

Answer 1

some minor things,

when you use showNextQuestion you should split up that method into the three sub-methods to clarify what you're doing:

private void showNextQuestion(){
 currentQuestion = getNextQuestion();
 showQuestion(currentQuestion );
 startCountDown();
}

that would make showNextQuestionto a mere delegation-method and it would be very clearly what this method does.

and if you did it this way you can re-use these methods during onCreate:

public void onCreate(Bundle savedInstanceState) {
 ...
 if (savedInstanceState == null) {
 loadQuestionsFromDb();
 }else{
 loadQuestionsFromSavedInstance(savedInstanceState); 
 } 
 showQuestion(currentQuestion); //as mentioned above
 if (answered) {
 //FIXME you don't show current score
 showSolution();
 } else {
 startCountDown();
 }
 ...
}

remove the counting part of checkAnswer into a seperate methode - thus makes the checkAnswermethod a clean delegation method

private void checkAnswer() {
 answered = true; 
 countDownTimer.cancel();
 score = score + countCurrentScore();
 textViewScore.setText("Score: " + score); 
 showSolution();
}

there are also some cosmetic things, maybe you should remove the blanks or shorten code when possible

if (timeLeftInMillis < 10000) {
 textViewCountDown.setTextColor(Color.RED);
} else {
 textViewCountDown.setTextColor(textColorDefaultCd);
}

into

textViewCountDown.setTextColor(timeLeftInMillis<10000?Color.RED:textColorDefaultCd); 

but thats no show stopper (there are some more issues of this type - but nevermind)

• RoToRa: I'd like to disagree on "you should remove the blanks or shorten code when possible". Readablity/understandability goes before shortness. The conditional expression would be much better readable with spaces and on a separate line: Color color = timeLeftInMillis < 10000 ? Color.RED : textColorDefaultCd;
• Martin Frank: uhm - i tried to point out that these changes are cosmetic proposals and depend on the OPs own view of things - i said maybe to emphase this personal view of mine and even added the no show stopper-part ^_^ @RoToRa - in my opinion i do not see any need to first get the color and then second set the text color with it - i for myself would simply set the text color directly... but what do i know ^^