2
\$\begingroup\$

I'm a beginner and have programmed a Java quiz game. However, it is currently way too long and there is probably a more efficient way.

 import java.util.Scanner;
import java.util.Random; 
class Wwtbam{
  public static void main (String args[]){
    Scanner scanner = new Scanner(System.in);
    Random random = new Random(); 
    int randomnum=0;
    int num1, num2, num3, num4 = 0; 
    String lifeline1= "1";
    String lifeline2="2";
    String lifeline3="3"; 
    String a = "a";
    String b = "b";
    String c = "c";
    String d = "d";
    String guess = "0";
    String guess1="0"; 
    String guess2="0";
    String guess3 = "0";
    String name;
    String finalanswer = "0";
    String x = "yes";
    String y = "no";
    String one = "What does NBA most commonly stand for? \n A. National Basketball Association \n B. New Brunswick Accounting \n C. Nevada Bowling Association \n D. Nile Boating Associates \n 1. 50/50 \n 2. Poll the audience \n 3. Ask a friend";
    String two = "Which country invented hockey? \n A. Iceland \n B. Canada \n C. Australia \n D. China \n 1. 50/50 \n 2. Poll the audience \n 3. Call a friend";

    System.out.println("What is your name?"); 
    name = scanner.nextLine();
    System.out.println("Welcome to Who Wants to be a Millionaire! As I'm sure you know, you will have to answer 13 questions correctly in a row. \n But you have 3 lifelines to work with: 50/50, poll the audience, or ask a friend \n So, " + name+ " lets begin! You have a chance to win ONE MILLION DOLLARS!!! \n\n\n\n\n");

    randomnum = random.nextInt(2)+1;
    if (randomnum ==1) {
      do {
        do{ 
          System.out.println(one);
          guess=scanner.nextLine();
          if ((!guess.equalsIgnoreCase(a) && !guess.equalsIgnoreCase(b) && !guess.equalsIgnoreCase(c) && !guess.equalsIgnoreCase(d)&&!guess.equalsIgnoreCase(lifeline1)&&!guess.equalsIgnoreCase(lifeline2)&&!guess.equalsIgnoreCase(lifeline3))) {
            System.out.println("Invalid Input. Please Try Again. \n\n");
          }
        } while(!guess.equalsIgnoreCase(a) && !guess.equalsIgnoreCase(b) && !guess.equalsIgnoreCase(c) && !guess.equalsIgnoreCase(d)&&!guess.equalsIgnoreCase(lifeline1)&&!guess.equalsIgnoreCase(lifeline2)&&!guess.equalsIgnoreCase(lifeline3));
        System.out.println("Is this your final answer? (yes or no)");
        finalanswer=scanner.nextLine(); 
      } while (!finalanswer.equalsIgnoreCase(x)); 

      if (guess.equalsIgnoreCase(a)) {
        System.out.println("You got it right! Congrats!");
      }
      else if ((guess.equalsIgnoreCase(b)) || (guess.equalsIgnoreCase(c)) || (guess.equalsIgnoreCase(d))) {
        System.out.println("Sorry you got it wrong :( \n Better luck next time");
      }
    }

   if (guess.equals(lifeline1)){
      System.out.println("You chose the 50/50 lifeline!");
      do{
        do{
          System.out.println("Your choices are now between: \n A. National Basketball Association \n C. Nevada Bowling Association");
          guess1 = scanner.nextLine(); 
          if ((!guess1.equalsIgnoreCase(a) && !guess1.equalsIgnoreCase(b) && !guess1.equalsIgnoreCase(c) && !guess1.equalsIgnoreCase(d)&&!guess1.equalsIgnoreCase(lifeline1)&&!guess1.equalsIgnoreCase(lifeline2)&&!guess1.equalsIgnoreCase(lifeline3))) {
            System.out.println("Invalid Input. Please Try Again. \n\n");
          }
        } while(!guess1.equalsIgnoreCase(a) && !guess1.equalsIgnoreCase(b) && !guess1.equalsIgnoreCase(c) && !guess1.equalsIgnoreCase(d)&&!guess1.equalsIgnoreCase(lifeline1)&&!guess1.equalsIgnoreCase(lifeline2)&&!guess1.equalsIgnoreCase(lifeline3));

        System.out.println("Is this your final answer? (yes or no)");
        finalanswer=scanner.nextLine();

      } while(!finalanswer.equals(x));

      if (guess1.equals(a)) {
        System.out.println("You got it right!!! Congrats " + name+ " !! \n\n");
      }
      else {
        System.out.println("Sorry you got it wrong :("); 
      }
    }

    else if (guess.equalsIgnoreCase(lifeline2)) {
      System.out.println("You chose the poll the audience lifeline!");

      num1 = random.nextInt(10)+80;
      num2 = random.nextInt(6)+1;
      num3 = random.nextInt(5)+1;
      num4 = 100 - (num1+num2+num3); 
      do{
        do{
          System.out.println("The audience, out of 100 people said: \n A. National Basketball Association: "+ (num1)+ " \n B. New Brunswick Accounting: "+num2+ "\n C. Nevada Bowling Association: "+num3+" \n D. Nile River Boating: "+num4 + "\n\n Enter your new guess:");
          guess2 = scanner.nextLine(); 
          if (!guess2.equalsIgnoreCase(a) && !guess2.equalsIgnoreCase(b) && !guess2.equalsIgnoreCase(c) && !guess2.equalsIgnoreCase(d)&&!guess2.equalsIgnoreCase(lifeline1)&& !guess2.equalsIgnoreCase(lifeline2)&& !guess2.equalsIgnoreCase(lifeline3)) {
            System.out.println("Invalid Input. Please Try Again. \n\n");
          }
        } while(!guess2.equalsIgnoreCase(a) && !guess2.equalsIgnoreCase(b) && !guess2.equalsIgnoreCase(c) && !guess2.equalsIgnoreCase(d)&&!guess2.equalsIgnoreCase(lifeline1)&& !guess2.equalsIgnoreCase(lifeline2)&& !guess2.equalsIgnoreCase(lifeline3));

        System.out.println("Is this your final answer? (yes or no)");
        finalanswer=scanner.nextLine();

      } while(!finalanswer.equals(x));
      if (guess2.equals(a)) {
        System.out.println("You got it right!!! Congrats " + name+ " !! \n\n");
      }
      else {
        System.out.println("Sorry you got it wrong :("); 
      }
    }


    else if(guess.equalsIgnoreCase(lifeline3)) {
      System.out.println("You chose the call a friend lifeline!");
      do{
        System.out.println("Your friend says: Oh it's definitely A! I grew up watching the National Basketball Association and playing basketball!");
        do{ 
          System.out.println("What is your answer?");
          guess3 = scanner.nextLine(); 
          if ((!guess3.equalsIgnoreCase(a) && !guess3.equalsIgnoreCase(b) && !guess3.equalsIgnoreCase(c) && !guess3.equalsIgnoreCase(d)&& !guess3.equalsIgnoreCase(lifeline1)&& !guess3.equalsIgnoreCase(lifeline2)&&!guess3.equalsIgnoreCase(lifeline3))) {
            System.out.println("Invalid Input. Please Try Again. \n\n");
          }
        } while(!guess3.equalsIgnoreCase(a) &&!guess3.equalsIgnoreCase(b)&& !guess3.equalsIgnoreCase(c) && !guess3.equalsIgnoreCase(d)&& !guess3.equalsIgnoreCase(lifeline1)&& !guess3.equalsIgnoreCase(lifeline2)&& !guess3.equalsIgnoreCase(lifeline3));

        System.out.println("Is this your final answer? (yes or no)");
        finalanswer=scanner.nextLine();

      } while(!finalanswer.equals(x));
      if (guess3.equals(a)) {
        System.out.println("You got it right!!! Congrats " + name+ " !! \n\n");
      }
      else {
        System.out.println("Sorry you got it wrong :("); 
      }
    }



    else if (randomnum==2){
      do {
        do{ 
          System.out.println(two);
          guess=scanner.nextLine();
          if ((!guess.equalsIgnoreCase(a) && !guess.equalsIgnoreCase(b) && !guess.equalsIgnoreCase(c) && !guess.equalsIgnoreCase(d)&&!guess.equalsIgnoreCase(lifeline1)&&!guess.equalsIgnoreCase(lifeline2)&&!guess.equalsIgnoreCase(lifeline3))) {
            System.out.println("Invalid Input. Please Try Again. \n\n");
          }
        } while(!guess.equalsIgnoreCase(a) && !guess.equalsIgnoreCase(b) && !guess.equalsIgnoreCase(c) && !guess.equalsIgnoreCase(d)&&!guess.equalsIgnoreCase(lifeline1)&&!guess.equalsIgnoreCase(lifeline2)&&!guess.equalsIgnoreCase(lifeline3));
        System.out.println("Is this your final answer? (yes or no)");
        finalanswer=scanner.nextLine(); 
      } while (!finalanswer.equalsIgnoreCase(x)); 

      if (guess.equalsIgnoreCase(b)) {
        System.out.println("You got it right! Congrats!");
      }
      else if((guess.equals(a)) || (guess.equals(c)) || (guess.equals(d))) {
        System.out.println("Sorry you got it wrong :( \n Better luck next time");
      }

      else if(guess.equalsIgnoreCase(lifeline1)) {
        System.out.println("You chose the 50/50 lifeline!");
        do{
          do{
            System.out.println("Your choices are now between: \n A. Iceland \n B. Canada");
            guess1 = scanner.nextLine(); 
            if ((!guess1.equalsIgnoreCase(a) && !guess1.equalsIgnoreCase(b) && !guess1.equalsIgnoreCase(c) && !guess1.equalsIgnoreCase(d)&&!guess1.equalsIgnoreCase(lifeline1)&&!guess1.equalsIgnoreCase(lifeline2)&&!guess1.equalsIgnoreCase(lifeline3))) {
              System.out.println("Invalid Input. Please Try Again. \n\n");
            }
          } while(!guess1.equalsIgnoreCase(a) && !guess1.equalsIgnoreCase(b) && !guess1.equalsIgnoreCase(c) && !guess1.equalsIgnoreCase(d)&&!guess1.equalsIgnoreCase(lifeline1)&&!guess1.equalsIgnoreCase(lifeline2)&&!guess1.equalsIgnoreCase(lifeline3));

          System.out.println("Is this your final answer? (yes or no)");
          finalanswer=scanner.nextLine();

        } while(!finalanswer.equals(x));
        if (guess1.equals(b)) {
          System.out.println("You got it right!!! Congrats " + name+ " !! \n\n");
        }
        else {
          System.out.println("Sorry you got it wrong :("); 
        }
      }

      else if(guess.equalsIgnoreCase(lifeline2)) {
        System.out.println("You chose the poll the audience lifeline!");

        num1 = random.nextInt(10)+20;
        num2 = random.nextInt(10)+55;
        num3 = random.nextInt(5)+3;
        num4 = 100 - (num1+num2+num3); 
        do{
          do{
            System.out.println("The audience, out of 100 people said: \n A. Iceland: "+ (num1)+ " \n B. Canada: "+num2+ "\n C. Australia: "+num3+" \n D. China: "+num4 + "\n\n Enter your new guess:");
            guess2 = scanner.nextLine(); 
            if (!guess2.equalsIgnoreCase(a) && !guess2.equalsIgnoreCase(b) && !guess2.equalsIgnoreCase(c) && !guess2.equalsIgnoreCase(d)&&!guess2.equalsIgnoreCase(lifeline1)&& !guess2.equalsIgnoreCase(lifeline2)&& !guess2.equalsIgnoreCase(lifeline3)) {
              System.out.println("Invalid Input. Please Try Again. \n\n");
            }
          } while(!guess2.equalsIgnoreCase(a) && !guess2.equalsIgnoreCase(b) && !guess2.equalsIgnoreCase(c) && !guess2.equalsIgnoreCase(d)&&!guess2.equalsIgnoreCase(lifeline1)&& !guess2.equalsIgnoreCase(lifeline2)&& !guess2.equalsIgnoreCase(lifeline3));

          System.out.println("Is this your final answer? (yes or no)");
          finalanswer=scanner.nextLine();

        } while(!finalanswer.equals(x));
        if (guess2.equals(b)) {
          System.out.println("You got it right!!! Congrats " + name+ " !! \n\n");
        }
        else {
          System.out.println("Sorry you got it wrong :("); 
        }
      }


      else if(guess.equalsIgnoreCase(lifeline3)) {
        System.out.println("You chose the call a friend lifeline!");
        do{
          System.out.println("Your friend says: Oh I think its B? I grew up in the U.S. but I remember the Canadians always winning the Olympic Gold Medal and lots of ceremonies for hockeys anniversaries. B is my best guess.");
          do{ 
            System.out.println("What is your answer?");
            guess3 = scanner.nextLine(); 
            if ((!guess3.equalsIgnoreCase(a) && !guess3.equalsIgnoreCase(b) && !guess3.equalsIgnoreCase(c) && !guess3.equalsIgnoreCase(d)&& !guess3.equalsIgnoreCase(lifeline1)&& !guess3.equalsIgnoreCase(lifeline2)&&!guess3.equalsIgnoreCase(lifeline3))) {
              System.out.println("Invalid Input. Please Try Again. \n\n");
            }
          } while(!guess3.equalsIgnoreCase(a) &&!guess3.equalsIgnoreCase(b)&& !guess3.equalsIgnoreCase(c) && !guess3.equalsIgnoreCase(d)&& !guess3.equalsIgnoreCase(lifeline1)&& !guess3.equalsIgnoreCase(lifeline2)&& !guess3.equalsIgnoreCase(lifeline3));

          System.out.println("Is this your final answer? (yes or no)");
          finalanswer=scanner.nextLine();

        } while(!finalanswer.equals(x));
        if (guess3.equals(b)) {
          System.out.println("You got it right!!! Congrats " + name+ " !! \n\n");
        }
        else {
          System.out.println("Sorry you got it wrong :("); 
        }
      }
    }
  }
  }
\$\endgroup\$
  • \$\begingroup\$ Hi Adam, I see that you already have a great code review here and if I posted another I would largely be repeating the existing review. So what I'd like to offer is this - if it would be helpful for you to see some of the concepts applied to your code I have taken your code and modified it and I would be happy to post it if you would like. Given that you're a beginner you may not understand all of the code immediately so you may have to do some research and learning before it all makes sense to you. \$\endgroup\$ – D.B. Oct 5 '17 at 1:27
1
\$\begingroup\$

As I stated in comments, the review by Sharon Ben Asher is great. I don't want to repeat everything already said in that review. Since Adam Lam has asked me to post code that illustrates the improvements mentioned in the review the majority of my review/answer is going to be code.

Note that the code below is not a complete implementation of all of the features of the game which this program is based on. I have included many comments within the code to help illustrate how things are working and what improvements could be made in the future. I tried to stay as close to the original presentation/view as I could.

Adam, As I mentioned in my comment on the review by Sharon Ben Asher, you should consider doing some additional research and reading. I think you could benefit from going through The Java Tutorials to learn about topics like OOP Concepts, Classes and Objects, Collections framework, and other essentials like Basic I/O. The code below uses the MVC Pattern to separate the data model from the presentation of that data and the logic of the gameplay, but there many other patterns that are useful in different situations.

With all of that said, here is the code...

This is the controller class:

package gameRefactor;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Random;

/**
 * This class is the controller of the game;
 * it coordinates the data model and view.
 */
public class WwtbamController {

    private static final Random random = new Random();
    private static final List<LifeLine> lifeLines = new ArrayList<>();
    //TODO: This is a basic example, you could have more than one view in which case you 
    // should use the Factory Pattern to determine which one to use at runtime. For example
    // perhaps someday you wish to build a GUI (Graphical User Interface) in addition to 
    // your text based interface. The practice of coding to an interface will make this easier.
    private static final View view = new WwtbamView();

    /**
     * Determines whether user input is valid based on whether the
     * input was parsed into a valid answer selection or lifeline given
     * the context of the situation.
     * @param question the currently active question; used to validate whether
     *          the user has entered a valid answer selection.
     * @param token a representation of user input as one of the answers to the question
     * @param lifeline a representation of user input as one of the lifeline options
     * @return true if the user input is valid, false otherwise.
     */
    private static boolean isInputValid(Question question, AnswerToken token, LifeLine lifeline){
        if(question.getAnswerTokens().contains(token)){
            return true;
        }

        return lifeline != null;
    }

    /**
     * Performs the logic necessary to use a lifeline
     * @param question the currently active question
     * @param lifeLine the lifeline selected for use
     */
    private static void useLifeLine(Question question, LifeLine lifeLineUsed){
        lifeLines.remove(lifeLineUsed);

        switch(lifeLineUsed){

        case CALL_A_FRIEND:
            //TODO: this is a very simple placeholder, you should replace this
            //      with whatever logic you want to use for generating your 
            //      "Call A Friend" results.
            view.displayCallFriendResults();
            break;
        case FIFTY_FIFTY:
            useFiftyFiftyLifeLine(question);
            break;
        case POLL_AUDIENCE:
            //TODO: this is a very simple placeholder, you should replace this
            //      with whatever logic you want to use for generating your 
            //      "Poll The Audience" results.
            view.displayAudienceResults();
            break;
        default: 
            System.err.println("ERROR Unknown lifeline; this should never happen!");
        }
    }

    /**
     * Executes the business logic of using the 50/50 lifeline by removing
     * 2 random incorrect answers from the specified question.
     * @param question the question whose answers are to be modified.
     */
    private static void useFiftyFiftyLifeLine(Question question){
        for(int i=0; i<2; i++){
            removeRandomIncorrectAnswer(question);
        }
    }

    /**
     * Removes a single random incorrect answer from the specified question.
     * @param question the question from which an answer will be removed.
     */
    private static void removeRandomIncorrectAnswer(Question question){
        List<Answer> incorrectAnswers = question.getIncorrectAnswers();
        int randomIndex = random.nextInt(incorrectAnswers.size());
        Answer randomAnswer = incorrectAnswers.get(randomIndex);

        question.removeAnswer(randomAnswer);
    }

    /**
     * Loads the questions into the appropriate data model
     * @return a list of all loaded questions.
     */
    private static List<Question> loadQuestions(){
        //TODO: In the game show the questions are categorized by difficulty
        //      so a List is not the most appropriate solution in the long run.
        List<Question> questions = new ArrayList<>();

        //TODO: Eventually you probably want to store question information in one or more files
        //      but for now they are hard-coded. If you do store question data in files then you will need to
        //      validate the data as well - for example if you want each question to have exactly 4 
        //      answers you will need to decide how to handle the situation where there are more or less
        //      than 4 answers specified in the data file. You could do that here in the controller class
        //      or split the responsibility between this class and the Question class.
        questions.add(new Question("What does NBA most commonly stand for?", AnswerToken.A,
                        new Answer(AnswerToken.A, "National Basketball Association"),
                        new Answer(AnswerToken.B, "New Brunswick Accounting"),
                        new Answer(AnswerToken.C,"Nevada Bowling Association"),
                        new Answer(AnswerToken.D,"Nile Boating Associates")));
        questions.add(new Question("Which country invented hockey?", AnswerToken.A,
                        new Answer(AnswerToken.A, "Iceland"),
                        new Answer(AnswerToken.B, "Canada"),
                        new Answer(AnswerToken.C, "Australia"),
                        new Answer(AnswerToken.D, "China")));
        return questions;
    }

    //TODO: As a general note someone playing the TV game show
    //      can decide to "walk away" at any time and keep the winnings
    //      that they have earned up to that point. This feature is not
    //      implemented.

    //TODO: In the game show there is the concept of a "Safety Net" 
    //      at which point the player can't leave with less than a 
    //      specific amount of money. This feature is also not yet
    //      implemented.
    public static void main(String[] args) {
        lifeLines.addAll(Arrays.asList(LifeLine.values()));     
        List<Question> questions = loadQuestions();

        view.displayIntro();

        int randomIndex = -1;

        //TODO: For now this loop simply continues until there are no more questions,
        //      but in the long run you will need to modify the logic to end when
        //      the player has successfully answered the $1M question.
        while(!questions.isEmpty()){
            randomIndex = random.nextInt(questions.size());
            //TODO: This is ok for a preliminary version of the game
            //      However, eventually you may want the user to be able to
            //      start over if they lose without having to re-run the program.
            //      In this case you don't want to modify the questions in this way,
            //      you would want to make a temporary copy of each question as you go
            //      and modify that.
            Question randQuestion = questions.get(randomIndex);
            boolean isFinalAnswer = false;

            while(!isFinalAnswer){
                view.displayQuestion(randQuestion);
                view.displayLifeLines(lifeLines);
                UserResponse questionResponse = view.getUserResponseToQuestion(lifeLines);

                //Validate input - the user should have entered an answer or lifeline
                //  If that is not the case, display an error and ask for more input
                //  until the response is valid.
                while( !isInputValid(
                        randQuestion, 
                        questionResponse.getAnswerToken(),
                        questionResponse.getLifeline())){
                    view.displayErrorMessage("Invalid Input. Please Try Again. \n\n");
                    questionResponse = view.getUserResponseToQuestion(lifeLines);
                }

                //When the user selects a lifeline use that lifeline.
                if(questionResponse.getLifeline() != null){
                    useLifeLine(randQuestion, questionResponse.getLifeline()); 
                }else{
                    //The user must have entered an answer so
                    //  ask if that's their final answer
                    isFinalAnswer = view.getIsFinalAnswer();
                    if(isFinalAnswer){
                        //If the player gave a final answer, check to see if it's correct
                        boolean rightAnswer = randQuestion.isCorrectAnswer(questionResponse.getAnswerToken());
                        if(rightAnswer){
                            //If the final answer was correct, let the player know.
                            view.displayOneLineMessage("You got it right! Congrats!");
                            //TODO: for now it is necessary to ensure that you don't ask the same question twice;
                            //      However, in the long run questions should be categorized and only one from
                            //      each category will be asked per game.
                            questions.remove(randomIndex);
                        }else{
                            //When the final answer is not correct end the game.
                            view.displayOneLineMessage("Sorry, that answer was incorrect. The game is now over. Please play again.");
                            return;
                        }
                    }
                }
            }
        }
        view.closeOpenResources();
    }

}

Here is the implementation of a View:

package gameRefactor;

import java.util.List;
import java.util.Scanner;

/**
 * This is the text-based implementation of the View interface. It obtains user
 * input from System.in and displays information by writing to System.out
 */
public class WwtbamView implements View {
    private static final Scanner scanner = new Scanner(System.in);

    public String getUserName(){
        displayOneLineMessage("What is your name?");
        return scanner.nextLine().trim();
    }

    public void displayQuestion(Question question){
        displayOneLineMessage(question.getText());
        for(Answer answer: question.getAnswers()){
            displayAnswer(answer);
        }
    }

    public void displayAnswer(Answer answer){
        StringBuilder builder = new StringBuilder();
        builder.append(answer.getToken().name());
        builder.append(". ");
        builder.append(answer.getText());
        displayOneLineMessage(builder.toString());
    }

    public void displayLifeLines(List<LifeLine> lifeLines){
        if(lifeLines.isEmpty()){
            displayOneLineMessage("You have no remaining lifelines.");
        }
        for(int i = 0; i < lifeLines.size(); i++){
            displayOneLineMessage((i+1) + ". "+lifeLines.get(i));
        }
    }

    public void displayCallFriendResults(){
        displayOneLineMessage("Your friend says C is statistically the best answer to multiple choice questions.");
    }

    public void displayAudienceResults(){
        displayOneLineMessage("The audience is no help, they didn't even vote! How rude!");
    }

    public void displayOneLineMessage(String promptText){
        System.out.println(promptText);
    }

    /**
     * Tries to obtain an AnswerToken from the user input
     * @param input the text entered by the user (assumed already trimmed)
     * @return the AnswerToken matching the input or null if there is no matching token.
     */
    private AnswerToken getAnswerTokenFromInput(String input){
        input = input.toUpperCase();
        try{
            AnswerToken token = AnswerToken.valueOf(input);
            return token;
        }catch(IllegalArgumentException e){
            return null;
        }   
    }

    /**
     * Tries to obtain a LifeLife from the user input
     * @param a list of the lifelines available when the user made their selection
     * @param input the text entered by the user (assumed already trimmed)
     * @return the LifeLine chosen by the user
     */
    private LifeLine getLifeLineFromInput(List<LifeLine> lifelines, String input){
        try{
            //Get the life line selected adjusting for the fact that the user
            //  input is 1 to 3 while the array is indexed starting at 0
            int index = Integer.parseInt(input)-1;
            if(index >= 0 && index < lifelines.size())
                return lifelines.get(index);
        }catch(NumberFormatException e){
            //Nothing to do here, user entered a non-integer value.
        }
        return null;
    }

    public UserResponse getUserResponseToQuestion(List<LifeLine> lifelines){
        String userInput = scanner.nextLine().trim();

        //Try to get an answer token from the user
        AnswerToken token = getAnswerTokenFromInput(userInput);
        //Try to get a lifeline selection from the user
        LifeLine lifeLine = getLifeLineFromInput(lifelines, userInput);

        return new UserResponse(token, lifeLine);
    }

    /**
     * Asks the user whether their answer is final
     * @return true if the user indicates the answer is final, false otherwise.
     */
    public boolean getIsFinalAnswer(){
        String finalAnswer = null;
        do{
            displayOneLineMessage("Is this your final answer? (yes or no)");
            finalAnswer = scanner.nextLine().trim();
        }while(!isValidYesNoResponse(finalAnswer));

        finalAnswer = finalAnswer.trim();
        return "Yes".equalsIgnoreCase(finalAnswer) || "Y".equalsIgnoreCase(finalAnswer);
    }

    /**
     * Validates that the specified input represents a valid yes or no response.
     * @param input the string to analyze
     * @return true if the input is yes, y, no, n (case insensitive); false otherwise.
     */
    private boolean isValidYesNoResponse(String input){
        input = input.trim();
        return input != null && ("Yes".equalsIgnoreCase(input) || 
                "y".equalsIgnoreCase(input) || "No".equalsIgnoreCase(input)
                || "n".equalsIgnoreCase(input));
    }

    public void closeOpenResources(){
        scanner.close();
    }

    //TODO: You may want to move your prompt text to a file
    //      later, possibly multiple files if you plan to support
    //      multiple languages. This applies to all prompts, not just
    //      the constants that follow this comment.
    private static final String INTRO_PREAMBLE = "Welcome to Who Wants to be a Millionaire! As I'm sure you know, you will have to answer 13 questions correctly in a row. \n But you have 3 lifelines to work with: 50/50, poll the audience, or ask a friend \n So, ";
    private static final String INTRO_POSTAMBLE = " lets begin! You have a chance to win ONE MILLION DOLLARS!!! \n\n\n\n\n";

    public void displayIntro(){
        String name = getUserName();
        displayOneLineMessage(INTRO_PREAMBLE + name + INTRO_POSTAMBLE);
    }

    public void displayErrorMessage(String msg){
        displayOneLineMessage(msg);
    }
}

Here is the View interface:

package gameRefactor;

import java.util.List;

/**
 * This interface specifies the functionality that
 * all views must implement. It will allow you to 
 * more easily create additional views in the future
 * if you decide to do so.
 */
public interface View {
    public void displayErrorMessage(String message);

    public void displayIntro();

    public void displayQuestion(Question question);

    public void displayAnswer(Answer answer);

    public void displayLifeLines(List<LifeLine> lifeLines);

    public void displayCallFriendResults();

    public void displayAudienceResults();

    public void displayOneLineMessage(String promptText);

    public UserResponse getUserResponseToQuestion(List<LifeLine> lifelines);

    public boolean getIsFinalAnswer();

    public String getUserName();

    public void closeOpenResources();
}

Finally the data model classes and enumerations:

The UserResponse class:

package gameRefactor;

/**
 * This class represents the user's response to a question,
 * which could be either a lifeline selection or one of the
 * answers.
 */
public class UserResponse {
    private LifeLine lifeline;
    private AnswerToken answerToken;

    public UserResponse(){}

    public UserResponse(AnswerToken token, LifeLine lifeLineValue){
        lifeline = lifeLineValue;
        answerToken = token;
    }

    public LifeLine getLifeline() {
        return lifeline;
    }
    public void setLifeline(LifeLine lifeline) {
        this.lifeline = lifeline;
    }
    public AnswerToken getAnswerToken() {
        return answerToken;
    }
    public void setAnswerToken(AnswerToken answerToken) {
        this.answerToken = answerToken;
    }
}

The Question class:

package gameRefactor;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.Set;

/**
 * This class represents a single question that could be 
 * presented to the player.
 */
public class Question {
    private String text;
    private AnswerToken correctAnswer;
    private List<Answer> answers;

    public Question(){}

    //TODO: You may wish to restrict the number of answers to 4
    //      which can be done in a few different ways. If you do
    //      implement this restriction within this class
    //      make sure to alter the setAnswers method as well.
    public Question(String txt, AnswerToken rightAnswer, Answer ... possibleAnswers){
        this.text = txt;
        this.correctAnswer = rightAnswer;
        this.answers = new ArrayList<>();
        this.answers.addAll(Arrays.asList(possibleAnswers));
    }

    public Set<AnswerToken> getAnswerTokens(){
        Set<AnswerToken> tokens = new HashSet<>();
        for(Answer answer: answers){
            tokens.add(answer.getToken());
        }
        return tokens;
    }

    public void removeAnswer(int index){
        answers.remove(index);
    }

    public void removeAnswer(Answer ans){
        answers.remove(ans);
    }

    public List<Answer> getAnswers() {
        return new ArrayList<>(answers);
    }

    public void setAnswers(List<Answer> choices) {
        this.answers = choices;
    }

    public String getText() {
        return text;
    }

    public void setText(String text) {
        this.text = text;
    }

    public AnswerToken getCorrectAnswer() {
        return correctAnswer;
    }

    public void setCorrectAnswer(AnswerToken answer) {
        this.correctAnswer = answer;
    }

    public boolean isCorrectAnswer(AnswerToken ans){
        //TODO: It's probably a good idea to ensure that a 
        //      question never has a null correct answer, 
        //      but for now this will do
        return correctAnswer == null ? ans == null : correctAnswer.equals(ans);
    }

    public List<Answer> getIncorrectAnswers(){
        List<Answer> incorrectAnswers = new ArrayList<>();
        for(Answer ans: answers){
            if(!isCorrectAnswer(ans.getToken()))
                incorrectAnswers.add(ans);
        }
        return incorrectAnswers;
    }
}

The LifeLine enum:

package gameRefactor;

/**
 * This enum represents all possible lifelines and
 * their textual representations.
 */
//TODO: If you do decide to add multi-language support you
//      will have to change how you represent the lifelines
//      since their text will not be constant.
public enum LifeLine {
    FIFTY_FIFTY("50/50"),
    POLL_AUDIENCE("Poll the audience"),
    CALL_A_FRIEND("Call a friend");

    private String text;
    private LifeLine(String text){
        this.text = text;       
    }
    public String getText() {
        return text;
    }

}

The AnswerToken enum:

package gameRefactor;

/**
 * This enum represents the tokens a player can enter in order to 
 * select one of the answers that has been presented.
 */
public enum AnswerToken {
    A,B,C,D
}

The Answer class:

package gameRefactor;

/**
 * This class represents an answer to a Question
 */
public class Answer {
    private String text;
    private AnswerToken token;

    public Answer(){}
    public Answer(AnswerToken tkn, String txt){
        token = tkn;
        text = txt;
    }
    public AnswerToken getToken() {
        return token;
    }
    public void setToken(AnswerToken token) {
        this.token = token;
    }
    public String getText() {
        return text;
    }
    public void setText(String text) {
        this.text = text;
    }

    @Override
    public int hashCode() {
        final int prime = 31;
        int result = 1;
        result = prime * result + ((text == null) ? 0 : text.hashCode());
        result = prime * result + ((token == null) ? 0 : token.hashCode());
        return result;
    }

    @Override
    public boolean equals(Object obj) {
        if (this == obj)
            return true;
        if (obj == null)
            return false;
        if (getClass() != obj.getClass())
            return false;
        Answer other = (Answer) obj;
        if (text == null) {
            if (other.text != null)
                return false;
        } else if (!text.equals(other.text))
            return false;
        if (token == null) {
            if (other.token != null)
                return false;
        } else if (!token.equals(other.token))
            return false;
        return true;
    }

}
\$\endgroup\$
2
\$\begingroup\$

Break code into smaller enclosed pieces

The code may be fairly simple but it doesn't look so. it is hard to follow the logic of such long, meandering code. They have a name for such code: "spaghetti" code because with all the flow paths in one place, it reminds of spaghetti noodles that are entangled together in a mess.

In order to clarify and organize, you should separate this into smaller pieces of code that do small enclosed tasks and put these in separate methods. This process can (and should) be iterated, braking the smaller pieces into even smaller ones, for the sole purple of clarity and brevity. it is not uncommon to see methods containing just one line of code.

Break code into reuseable pieces

Another motivation for breaking up long code into smaller pieces is reuseability. The game consists of asking 13 questions. the process that needs to be done in each question is identical: present the question, get the answer, validate it, check if it is correct, offer remaining lifelines, etc etc . all that changes is the contents: the text of the question and possible answers. So instead of asking 13 times if (randomnum ==1/2/3/4/5...) { and nearly duplicating the code inside the if statements, create a method that handles the process of asking a question and getting the answer . the method should receive the "changing" part of the code: the text of the question, what is the correct answer, and perhaps few more pieces of data.

Organize your Data

So far, I mentioned programming rules that are universal to every paradigm and are considered "common sense" in the sense that all they demand is to be clear and organized. Now I will talk about a rule that is part of Object Oriented paradigm: data encapsulation .

Let us consider the concept of a question: it has the following properties: question text, 4 possible answers, one of which is the correct one. at the beginning of processing of a question, all possible answers are displayed, but during the processing, some of them should be "hidden" (due to usage of lifelines) so perhaps it is a good idea to add a isRelevant property to each answer.

so according to the data encapsulation rule, the concept of a Question should be a separate class that holds the relevant properties and is also responsible for doing the tasks that involve accessing and modifying the properties. only the methods of a Question class should directly access the properties. why? because you may decide to put the question and answer text in a file, or you may decide to support other languages... the idea is that the data may change, and when the data is encapsulated in one class, you know you need to change only that class. So a Question class should have a private String text instance variable and a public String getText() method that returns the text. it may contains just return text; but like we said, you have one place which is responsible for reading the question text, and that allows for fexibility of future changes.

So, based on all the above, Here is a suggestion for high level break up of code into methods:

name = getUserName();
startGame();  // welcome msg, explain rules
do {
  Question question = getNextQuestion();  // random num
  presentQuestion(question.getText());
  String userResponse = getUserResponse();
  if isValid(userResponse) {
    if (userisGuessingAnswer(userResponse)) {
      if (question.isCorrectAnswer(userResponse)) {
      ...
      } else {
      ...
      }
    }
    if (userisAskingForLifeline(userResponse)) {
      Lifeline ll = getLifeline(userResponse);
      if (ll.isAvaiable()) {
        String guess = ll.doLifeline(question);  // lifeline may modify answers inside question
        ...
      } else {
      ...
      }
    }
  }
} while (!endGame());

Declare variables in the smallest scope

In addition, you have all your variables declared in the beginning of the one method you have. Some of these are used in particular places only. For example, guess2 is only used when lifeline2 is used. another OO paradigm rule says that variables should be declared inside the smallest scope that they are used. why? 1) it makes for easier reading of the code when one knows where a variable is used and where it is not. 2) it makes for safer modifications in the variable 3) sometimes you can use the same name on different local variables instead of having guess1/guess2 and so on:

if (userChoseLifeline1()) {
  String guess = scanner.nextLine(); // scope is inside the 1st if block
  if (question.isCorrectGuess(guess) {
  }
}
if (userChoseLifeline2()) {
  String guess = scanner.nextLine();  // scope is inside the 2nd block
  if (question.isCorrectGuess(guess) {
  }
}
\$\endgroup\$
  • \$\begingroup\$ Great code review. I think the only things I would add are some additional links for further reading: Design Patterns and the MVC pattern in particular, and The Java Tutorials for things like OOP Concepts, Classes and Objects, Collections framework, essentials like Basic I/O, etc. \$\endgroup\$ – D.B. Oct 5 '17 at 1:17
-1
\$\begingroup\$

According to me the code is fairly simple and you are using only if else and do while, the things make your code lengthy is your questions and answers. I saw that you used many strings and integer try using arrays if I’m right. Hope this helps a bit

\$\endgroup\$
  • 2
    \$\begingroup\$ It may be very helpful if you provide a few examples of how the code can be improved. Similarly, it's important to point to specific issues rather than use broad terms. Try take a look at answers to other questions. Just my five cents. \$\endgroup\$ – Igor Soloydenko Oct 4 '17 at 2:54
  • \$\begingroup\$ Sorry im new to stakexchage and pretty new to programming. My teacher said that visit stakexchange, here I can help others and get helped even. It was a simple suggestion because I know only a little bit about coding and I’m here to learn. If you have time can you answer one of my question I need help. \$\endgroup\$ – Inder Aujla Oct 4 '17 at 3:04
  • \$\begingroup\$ the code uses redundant nested do blocks, duplicated validation checks, duplicated String answers..... not to mention all the logic is cramped into one method. this code looks simple ? \$\endgroup\$ – Sharon Ben Asher Oct 4 '17 at 7:18

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.