HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Simple spin game

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
gamesimplespin

Problem

I see many many programs that are written out of the main method. But most of my programs are written in main() and access some methods from other classes.

Am I programming in the correct way?

main.java:

```
import java.util.Scanner;
import games.Spin;

public class Mains {
//Declaring Scanner object as console.
static Scanner console = new Scanner(System.in);
//Declaring Spin game object as spin
static Spin spin = new Spin();

//Available games array
static String[] content = {"spin", "tof"};
//Navigation options array
static String[] navigateGame = {"start", "back"};

public static void main (String[] args) {

// Setting the default number for inputNumber
int inputNumber = 0;

// Setting the defult value for inputString
String inputString = "";

// The main game loop
boolean game = true;

// Spin game loop
boolean spinGame = false;

// Truth or false game loop
boolean tofGame = false;

System.out.print("Welcome! Please select a game: ");
for (int i = 0; i < content.length; i++) {
System.out.print(content[i]);
if (i < content.length -1) {
System.out.print(", ");
}
}

//New line..
System.out.println();

//The main game loop
while (game) {

//This is the input we will enter to the console.
inputString = console.nextLine();

//If we are not in any of the games..
if (!spinGame && !tofGame) {
//Looping through the array
for (String s : content) {
//If the entered input, contains any word that is in the array, enter switch statement, to find out which word was it.
if (inputString.equalsIgnoreCase(s)) {
//Entering switch statement to find out which of the values matched in the array.

Solution

Having the entire program contained in one function is rather frowned upon by pretty much all camps. A function hundreds of lines long can be a real pain to fix, if ever you have to.

Add to that, you're using 4 flags to keep track of what you're doing. And it's just going to keep growing, if you add more games. The bigger the function gets, the more moving parts there are...the more stuff shares a scope...the easier it is for something to get modified by accident, or collide namewise, or any number of other things, and the easier it is for functionality to blend together, whether in the code or in your mind. If you break up the code into short, discrete chunks with decently descriptive names, it reduces overall complexity as well as cognitive load.

You'd do well to identify parts of the code that represent discrete operations, and split them off into their own functions. For example, in your main method, you have:

  • Code to prompt for a game choice



  • Code to ask the user what to do next in the current game



  • Code to run the "spin" game



  • Eventually, i presume, code to run the "tof" game



These chunks, at the very least, could be split off -- particularly since you currently use flags to make the states mutually exclusive anyway. Those flags you're using would disappear; the program would begin tracking its state implicitly, by what function it's in.

public class Mains {
    static Scanner console = new Scanner(System.in);

    // Navigation options array
    // i'm guessing this will be shared between spin and tof
    static String[] navigateGame = {"start", "back"};

    public static void main (String[] args) {
        String[] content = { "spin", "tof" };
        while (true) {
            printGameMenu(content);
            String choice = getChoice(content);

            switch (choice) {
                case "spin":
                    spinGame();
                    break;
                /* once you implement the TOF game...
                case "tof":
                    tofGame();
                    break;
                */
                default:
                    System.out.println("Could not find game!");
                    break;
            }
        }
    }

    public static void printGameMenu(String[] content) {
        System.out.print("Welcome! Please select a game: ");
        for (int i = 0; i < content.length; i++) {
            System.out.print(content[i]);
            if (i < content.length -1) {
                System.out.print(", ");
            }
        }
        System.out.println();
    }

    public static String getChoice(String[] options)
    {
        do {
            String inputString = console.nextLine();

            for (String s : options) {
                if (inputString.equalsIgnoreCase(s)) {

                    // BTW...rereading this Q&A, i noticed something.  In your code,
                    // when you've found a match, you use `inputString` -- the string
                    // entered by the user -- and you don't know whether it's
                    // lowercase.  This means even though you've found a match, it
                    // might not match any of your `switch` cases.
                    //
                    // The string in your options array is more under your control.
                    // You should probably either use that or stop trying to be
                    // case-agnostic, since it probably isn't working anyway.

                    return s;
                }
            }
            System.out.println("Invalid option.  Please choose from the listed options.");
        } while (true);
    }

    public static void spinGame()
    {
        Spin spin = new Spin();
        System.out.println("Welcome to the spin game!");
        System.out.println("Please enter 'start' to start and 'back' to go back.");

        while (true) {
            String inputString = getChoice(navigateGame);

            switch (inputString) {
                case "start":
                    System.out.println(spin.spinWheel());
                    break;
                case "back":
                    return;
                default:
                    // this should never happen
                    throw new RuntimeException("Unexpected: '" + inputString + "'");
            }
        }
    }
}


By the way, in your code, you call main(args) to go back to the game selection phase. Don't do that. That isn't a loop, it's recursion...and if you do it over and over, without returning, eventually your program is going to run out of stack space and crash. Not to mention, it makes exiting awkward; now you have to explicitly kill the app.

Code Snippets

public class Mains {
    static Scanner console = new Scanner(System.in);

    // Navigation options array
    // i'm guessing this will be shared between spin and tof
    static String[] navigateGame = {"start", "back"};

    public static void main (String[] args) {
        String[] content = { "spin", "tof" };
        while (true) {
            printGameMenu(content);
            String choice = getChoice(content);

            switch (choice) {
                case "spin":
                    spinGame();
                    break;
                /* once you implement the TOF game...
                case "tof":
                    tofGame();
                    break;
                */
                default:
                    System.out.println("Could not find game!");
                    break;
            }
        }
    }

    public static void printGameMenu(String[] content) {
        System.out.print("Welcome! Please select a game: ");
        for (int i = 0; i < content.length; i++) {
            System.out.print(content[i]);
            if (i < content.length -1) {
                System.out.print(", ");
            }
        }
        System.out.println();
    }


    public static String getChoice(String[] options)
    {
        do {
            String inputString = console.nextLine();

            for (String s : options) {
                if (inputString.equalsIgnoreCase(s)) {

                    // BTW...rereading this Q&A, i noticed something.  In your code,
                    // when you've found a match, you use `inputString` -- the string
                    // entered by the user -- and you don't know whether it's
                    // lowercase.  This means even though you've found a match, it
                    // might not match any of your `switch` cases.
                    //
                    // The string in your options array is more under your control.
                    // You should probably either use that or stop trying to be
                    // case-agnostic, since it probably isn't working anyway.

                    return s;
                }
            }
            System.out.println("Invalid option.  Please choose from the listed options.");
        } while (true);
    }

    public static void spinGame()
    {
        Spin spin = new Spin();
        System.out.println("Welcome to the spin game!");
        System.out.println("Please enter 'start' to start and 'back' to go back.");

        while (true) {
            String inputString = getChoice(navigateGame);

            switch (inputString) {
                case "start":
                    System.out.println(spin.spinWheel());
                    break;
                case "back":
                    return;
                default:
                    // this should never happen
                    throw new RuntimeException("Unexpected: '" + inputString + "'");
            }
        }
    }
}

Context

StackExchange Code Review Q#28060, answer score: 5

Revisions (0)

No revisions yet.