1
\$\begingroup\$

I am trying to re-factor (improve design of existing working code) with the following principals:

  • Switch statements removal.
  • Encapsulate Field
  • Extract Class
  • Extract Interface
  • Extract Method
  • Extract Subclass
  • Extract Super Class
  • Form Template Method - Before
  • Move Method - Before
  • Introduce Null Object
  • Replace Error Code with Exception
  • Replace Exception with Test
  • Nested Conditional with Guard
  • Replace Parameter with Explicit Method
  • Replace Temp with Query
  • Rename Variable or Method

Here is my code. Any help or suggestions?

Class: TriviaData

import java.util.ArrayList;

public class TriviaData
{
    private ArrayList<TriviaQuestion> data;

    public TriviaData()
    {
        data = new ArrayList<TriviaQuestion>();
    }

    public void addQuestion(String q, String a, int v, int t)
    {
        TriviaQuestion question = new TriviaQuestion(q,a,v,t);
        data.add(question);
    }

    public void showQuestion(int index)
    {
        TriviaQuestion q = data.get(index);
        System.out.println("Question " + (index +1) + ".  " + q.value + " points.");
        if (q.type == TriviaQuestion.TRUEFALSE)
        {
            System.out.println(q.question);
            System.out.println("Enter 'T' for true or 'F' for false.");
        }
        else if (q.type == TriviaQuestion.FREEFORM)
        {
            System.out.println(q.question);
        }
    }
public int numQuestions()
    {
        return data.size();
    }

    public TriviaQuestion getQuestion(int index)
    {
        return data.get(index);
    }
}

public class TriviaQuestion
{
    public static final int TRUEFALSE = 0;
    public static final int FREEFORM = 1;

    public String question;     // Actual question
    public String answer;       // Answer to question
    public int value;           // Point value of question
    public int type;            // Question type, TRUEFALSE or FREEFORM

    public TriviaQuestion()
    {
        question = "";
        answer = "";
        value = 0;
        type = FREEFORM;
    }

    public TriviaQuestion(String q, String a, int v, int t)
    {
        question = q;
        answer = a;
        value = v;
        type = t;
    }
}

Class TriviaGame

import java.io.*;
import java.util.Scanner;

public class TriviaGame
{
    public TriviaData questions;    // Questions

    public TriviaGame()
    {
    // Load questions
    questions = new TriviaData();
    questions.addQuestion("The possession of more than two sets of chromosomes is termed?",
        "polyploidy", 3, TriviaQuestion.FREEFORM);
    questions.addQuestion("Erling Kagge skiied into the north pole alone on January 7, 1993.",
        "F", 1, TriviaQuestion.TRUEFALSE);
    questions.addQuestion("1997 British band that produced 'Tub Thumper'",
        "Chumbawumba", 2, TriviaQuestion.FREEFORM);
    questions.addQuestion("I am the geometric figure most like a lost parrot",
        "polygon", 2, TriviaQuestion.FREEFORM);
    questions.addQuestion("Generics were introducted to Java starting at version 5.0.",
        "T", 1, TriviaQuestion.TRUEFALSE);
    }
// Main game loop
public static void main(String[] args)
{
    int score = 0;          // Overall score
    int questionNum = 0;    // Which question we're asking
    TriviaGame game = new TriviaGame();
    Scanner keyboard = new Scanner(System.in);
    // Ask a question as long as we haven't asked them all
    while (questionNum < game.questions.numQuestions())
    {
    // Show question
    game.questions.showQuestion(questionNum);
    // Get answer
    String answer = keyboard.nextLine();
    // Validate answer
    TriviaQuestion q = game.questions.getQuestion(questionNum);
    if (q.type == TriviaQuestion.TRUEFALSE)
    {
        if (answer.charAt(0) == q.answer.charAt(0))
        {
            System.out.println("That is correct!  You get " + q.value + " points.");
            score += q.value;
        }
        else
        {
            System.out.println("Wrong, the correct answer is " + q.answer);
        }
    }

else if (q.type == TriviaQuestion.FREEFORM)
    {
        if (answer.toLowerCase().equals(q.answer.toLowerCase()))
        {
            System.out.println("That is correct!  You get " + q.value + " points.");
            score += q.value;
        }
        else
        {
            System.out.println("Wrong, the correct answer is " + q.answer);
        }
    }
    System.out.println("Your score is " + score);
    questionNum++;
 }
 System.out.println("Game over!  Thanks for playing!");
 }
}
\$\endgroup\$
5
\$\begingroup\$
  1. It's better to use enum { TRUEFALSE, FREEFORM } instead of hard coding arbitrary int values.

  2. It's better to declare your variables as List<TriviaQuestion> data and only use ArrayList when you instantiate them. You then have more freedom later if you want to change the type of list you are using. In general you always want to declare variables with the most general superclass/interface type possible.

  3. Use meaningful variable names, not (String q, String a, int v, int t). Variables names are a form of documentation. On that same topic, instead of using the variable name "data" I would use "questions" or "questionList".

  4. I don't like the class TriviaData: it doesn't do anything more than using a plain List<TriviaQuestion>. You can just questions.add(new TriviaQuestion(...)).

  5. It's better not to have the empty constructor TriviaQuestion().

  6. Instead of making the members of TriviaQuestion public, it's better to keep them private and add getters (but no setters since they should only be set by the constructor).

Your bracket indentation is from C/C++. Your code will run nonetheless, but maybe you should use the Java conventions if you are switching to Java.

|improve this answer|||||
\$\endgroup\$
4
\$\begingroup\$

I think you missed a crucial refactoring: Replace Type Code With Subclasses.

questions.add(new FreeformQuestion(3,
    "The possession of more than two sets of chromosomes is termed?",
    "polyploidy"));
questions.add(new TrueFalseQuestion(1,
    "Erling Kagge skiied into the north pole alone on January 7, 1993.",
    false));

Then, the game loop should be vastly simplified:

for (TriviaQuestion q : game.questions) {
    boolean isCorrect = q.promptAndVerify(keyboard, System.out);
    if (isCorrect) {
        System.out.format("That is correct!  You get %d points.\n", q.getValue());
        score += q.getValue();
    } else {
        System.out.format("Wrong, the correct answer is %s\n", q.getAnswer());
    }
    System.out.format("Your score is %d\n", score);
}

By delegating the prompting and verification to the question, you gain a lot of flexibility. You could add question subclasses for Daily Doubles, multiple choice questions, timed questions, etc.

|improve this answer|||||
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.