2
\$\begingroup\$

I'm really new to programming, currently following several tutorials and guidelines. After a couple of days of studying, I've learnt the basics of Java and conditionals and took on my first program. It was written all out of my head.

The program is a sort of gambling machine. Each turn, the player has to guess a number out of 10 and if his number gets drawn he wins 50 credits. He must choose how much money to play, for how many turns and the stake for each turn.

I would like to receive some feedback regarding my code. What did I do wrong or some helpful tips/suggestions. Once I completed it, I keep having this feeling that the code is too long considering the simple task it does. How could I make it more compact?

import java.util.Random;
import java.util.Scanner;

/**
* Created by doublin on 3/3/16.
*/
public class Lottery {

public static void main(String[] args) {

    System.out.println("Welcome!");
    System.out.println("In this game you must guess a number between 1 and 10.");

    //******************************(START) MONEY LOADING     ***********************************************
    System.out.println("How much money you want to load in to the machine?");
    int currentMoney;
    Scanner moneyInput = new Scanner(System.in);
    while (!moneyInput.hasNextInt()) {
        moneyInput.next();
        System.out.println("Invalid amount.");
    }
    currentMoney = moneyInput.nextInt();
    //********************************(END) MONEY LOADING ************************************************

    //******************************(START) NUMBER OF PLAYS **********************************************
    System.out.println("How many times you want to try?");

    int maxTries;
    int currentTries = 0;
    Scanner userTries = new Scanner(System.in);
    while (!userTries.hasNextInt()) {
        userTries.next();
        System.out.println("Invalid choice.");
    }
    maxTries = userTries.nextInt();
    //*******************************(END) NUMBER OF PLAYS **********************************************

    //*******************************(START) STAKE CHOICE ***********************************************
    System.out.println("Choose a stake:");

    int stake;
    Scanner stakeInput = new Scanner(System.in);
    while (!stakeInput.hasNextInt()) {
        stakeInput.next();
        System.out.println("Invalid stake.");
    }
    stake = stakeInput.nextInt();
//***************************************(END) STAKE CHOICE  *************************************************
    int gameNumber = 1;
    while(currentTries < maxTries && currentMoney > 0) {

        System.out.println("Game #" + gameNumber);
        System.out.println("Now please, choose your number: ");

        currentMoney = currentMoney - stake;
        int userNumber;
        Scanner userChoice = new Scanner(System.in);
        while (!userChoice.hasNextInt()) {
            userChoice.next();
            System.out.println("Only numbers from 1 to 10 please.");
        }
        userNumber = userChoice.nextInt();

        Random random = new Random();
        int extractedNumber = random.nextInt((10-1) + 1) + 1;
        System.out.println("The extracted number is: " + extractedNumber);

        if (userNumber == extractedNumber) {
            currentMoney = currentMoney + 50;
            System.out.println("Congratulations! You've won 50 bucks!");
            System.out.println("Now you have " + currentMoney);
            System.out.println();
        }
            else {
                System.out.println("You were unlucky.");
                System.out.println("You have left " + currentMoney);
                System.out.println();
            }

        currentTries++;
        gameNumber++;

        if (currentMoney <= 0) {
            System.out.println("*** GAME OVER ***");
            System.out.println("Sorry, you lost all your money.");
        }
        if (currentTries == maxTries) {
            System.out.println("*** GAME OVER ***");
            System.out.println("You reached " + gameNumber + " games.");
            System.out.println("Money left: " + currentMoney);
        }
     }
  }
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Welcome to Code Review! This question is incomplete. To help reviewers give you better answers, please add sufficient context to your question. The more you tell us about what your code does and what the purpose of doing that is, the easier it will be for reviewers to help you. See also this meta question. \$\endgroup\$ – SuperBiasedMan Mar 4 '16 at 9:33
1
\$\begingroup\$

It's not a very fair betting machine, because the stake does nothing. You could bet 0 money and the machine would still payout 50 if you were correct. Similarly, you could stake 500 per round, and you'd lose even if you won!

Aside from that, you should split your code up into separate functions.

Take getting a number, for instance.

You managed to spot that they're three separate segments, by labelling them with a comment. But what you could do is just make them separate function calls:

System.out.println("How much money you want to load in to the machine?");
int currentMoney;
Scanner moneyInput = new Scanner(System.in);
while (!moneyInput.hasNextInt()) {
    moneyInput.next();
    System.out.println("Invalid amount.");
}
currentMoney = moneyInput.nextInt();

System.out.println("How many times you want to try?");

int maxTries;
int currentTries = 0;
Scanner userTries = new Scanner(System.in);
while (!userTries.hasNextInt()) {
    userTries.next();
    System.out.println("Invalid choice.");
}
maxTries = userTries.nextInt();

These two snippets differ in that currentTries is wedged in the second one, but we don't need it here - it's not useful for doing things to maxTries. They also differ in the variable written to, which we can use as return value and the strings displayed.

I'd make a function getNumber(Scanner inputSource, String initialMessage, String errorMessage).

public int getNumber(Scanner inputSource, String initialMessage, String errorMessage){
    System.out.println(initialMessage);
    while (!inputSource.hasNextInt()) {
        inputSource.next();
        System.out.println(errorMessage);
    }
    return inputSource.nextInt();
}

Directly integrating with your code, it would look like so:

//******************************(START) MONEY LOADING     ***********************************************
int currentMoney = getNumber(new Scanner(System.in), "How much money you want to load in to the machine?", "Invalid amount.");
//********************************(END) MONEY LOADING ************************************************

//******************************(START) NUMBER OF PLAYS **********************************************
int currentTries = 0;
int maxTries = getNumber(new Scanner(System.in), "How many times you want to try?", "Invalid choice.");
//*******************************(END) NUMBER OF PLAYS **********************************************

//*******************************(START) STAKE CHOICE ***********************************************
int stake = getNumber(new Scanner(System.in), "Choose a stake:", "Invalid stake.");
//***************************************(END) STAKE CHOICE  *************************************************

As a direct result, you no longer need all the //****** anymore.

int currentMoney = getNumber(new Scanner(System.in), "How much money you want to load in to the machine?", "Invalid amount.");
int currentTries = 0;
int maxTries = getNumber(new Scanner(System.in), "How many times you want to try?", "Invalid choice.");
int stake = getNumber(new Scanner(System.in), "Choose a stake:", "Invalid stake.");

You might want to fix it so that they all use the same scanner, but you'll have to test carefully there, because users might be capable of entering things like "10 20" which is two separate ints (they'd answer 2 questions at the same time).

\$\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.