Simple spin game

Question

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.
                        switch (inputString) {
                            //Spin game match
                            case "spin":
                                //Start spin game
                                spinGame = true;
                            break;

                            //Tof game match.
                            case "tof":
                                //Start tof game
                                tofGame = true;                        
                            break;                            
                        }
                        //Getting out of the for loop, so we won't get could not find game error after match found.
                        break;
                    } else {
                        //No match found, throw error.
                        System.out.println("Could not find game!");
                        //Break out of the loop.
                        break;
                    }
                }                
            }

            /*
            * Spin Game Start
            */


            //Setting up the text once user entered the game.

            if (spinGame) {
                System.out.println("Welcome to the spin game!");
                System.out.println("Please enter 'start' to start and 'back' to go back.");
            }

            /*
            * Main Spin Game loop
            */

            while (spinGame) {

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

                //Looping through the array for selected navigation option.
                for (String s : navigateGame) {
                    //Checking if the entered value contains any word inside the navgiation options array.
                    if (inputString.equalsIgnoreCase(s)) {
                        //Entering the switch statement to find out what word.
                        switch (inputString) {

                            //Case start game
                            //This will spin the wheel once, and tell results.
                            case "start":
                                //Printing the spinned value frm spinWheel().
                                System.out.println(spin.spinWheel());
                                //Setting the console to receive more inputs.
                                inputString = console.nextLine();
                            break;

                            //Case back
                            //This will go back to the lobby.
                            case "back":
                                //Quit spinGame.
                                spinGame = false;
                                //Back to the top of the method.
                                main(args);
                            break;
                        }
                    }
                }
            }
        }
    }
}

Spin.java:

package games;

public class Spin {

    public String spinWheel() {
        return "It works!";
    }
}

I did not make the spin part because I really need to know if I am doing well. Because I see most of the people are doing things in the different way, I hate sticking with a one way while I think it's incorrect.

Is there anything wrong with my way of coding and handling?

Answer 1

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.

Answer 2

Since nobody has mentioned it so far:

All your comments are bad.

They merely repeat what the code does. They provide no additional information whatsoever. As such, they are redundant, create visual clutter which makes the code harder to read, and they need to be updated whenever your code changes, otherwise they become out of sync with the code and provide false information.

All of these comments must be removed (yes, I really mean all comments).

Comments should only be used to explain why a piece of code is in place (or why it’s missing).

The only comment that goes in the right direction is the following:

// A good solution to not loop through all games when printing.

Unfortunately, it doesn’t go far enough: I still have no idea what the purpose of notFound is, and I suspect part of the reason is the variable name. Choose variable names that are concise yet specific in describing what they do. But you don’t use that variable anyway so its name doesn’t matter.

Answer 3

There are several things that you can improve in your code. Actually a good practice for your own benefit as a developer is plan before you do it. If you are planing to do a doghouse, you don't need a blueprint, but if you are building your house a blueprint looks good (Code Complete 2 reference).

When you do everything in the same class or method you are killing lots of good options that Object Oriented Programming give you. For example, if you need to extend your class, and just change a part of it, if everything is in the same method you will have to copy everything just to change maybe a line. More about overriding here. You should aim to have specialized function classes.

Also, you will improve readability and how clean is your code. It's not easy to figure out what your code is doing when you have a huge method. Method names should be really indicative of that the method will do or return.