7
\$\begingroup\$

Please let me know how I can make it better, especially with avoiding repetition without the need to replace the user guesses.

import java.util.Scanner;
public class Lottery {

    static int guess[] = new int[3],actual[]= new int[3];

    private static void aquireGuess()
    {
        Scanner keyboard = new Scanner(System.in);
        System.out.println("Time to play the lottery please choose three numbers ranging from zero to nine.");
        System.out.print("Please enter first number:");
        guess[0] = keyboard.nextInt();
        System.out.print("Please enter second number:");
        guess[1] = keyboard.nextInt();
        System.out.print("Please enter third number:");
        guess[2] = keyboard.nextInt();
        keyboard.close();
    }

    private static void generateLottery()
    {
        actual[0] = (int)(Math.random()*10);
        actual[1] = (int)(Math.random()*10);
        actual[2]= (int)(Math.random()*10);
    }

    private static void compare()
    {   
        if(guess[0]==actual[0]&&guess[1]==actual[1]&&guess[2]==actual[2])
        {
            System.out.println("Congradulations three matches in exact order!!! You win $1,000,000.");
            return;
        }
        int count = 0;
        for(int i=0;i<=2;i++)
        {
            for(int x=0;x<=2;x++)
            {
                if (actual[i]==guess[x])
                {
                    count++;
                    guess[x]=-1;
                    break;
                }
            }
        }
        switch(count)
                {
        case 0:System.out.println("Sorry better luck next time."); break;
        case 1:System.out.println("Good job. One match. You win $10."); break;
        case 2:System.out.println("Awsome you made two matches. You win $100."); break;
        case 3:System.out.println("Woohoo you matched all three. You win $1,000.");break;
                }
    }

    public static void main(String args[])
    {
        aquireGuess();
        generateLottery();
        System.out.println("Your guess were "+guess[0]+" "+guess[1]+" "+guess[2]);
        System.out.println("The lottery numbers were "+actual[0]+" "+actual[1]+" "+actual[2]);
        compare();
    }
}
\$\endgroup\$

2 Answers 2

6
\$\begingroup\$

Using method return types

Instead of relying on static int[] arrays, you should return the appropriate results from getting the user input and generating the lottery numbers so that their interaction is slightly clearer.

Validating user input

Scanner.nextInt() throws an InputMismatchException when the next input is not parsable as an integer. You should have a method that handles that safely. For example (prompted by @KonradMorawski's comment):

private static int getInteger(Scanner scanner) {
    System.out.println("Enter an integer:"); 
    while (!scanner.hasNextInt()) {
        System.out.println("Not an integer, please try again.");
        scanner.next();
    }
    return scanner.nextInt();
}

Comparing arrays

Arrays.equals(int[], int[]) can be used to compare two int[] arrays.

Using other types for comparison

You may also want to consider using other types to compare the lottery numbers. For example, a plain integer works as well:

private static int getNumber(int a, int b, int c) {
    return a * 100 + b * 10 + c;
}

edit:

try-with-resources

If you are on Java 7 and above, you should use try-with-resources for efficient handling of the underlying I/O resource:

public static void main(String[] args) {
    try (Scanner scanner = new Scanner(System.in)) {
        // ...
    }
}
\$\endgroup\$
2
  • 1
    \$\begingroup\$ "Scanner.nextInt() throws an InputMismatchException when the next input is not parsable as an integer. You should have a method that handles that safely" - in my opinion you are wrong here, best practice is to evaluate validity of user input (in this case simply checking Scanner.hasNextInt()) instead of using exceptions to control the flow (this is an antipattern). \$\endgroup\$ Commented Nov 2, 2015 at 14:03
  • 1
    \$\begingroup\$ @KonradMorawski right, I was brief in my answer but I do mean using hasNextInt() in conjunction with any additional prompts/validations in a method that takes in a Scanner instance and returns an int. Thanks for mentioning that! \$\endgroup\$
    – h.j.k.
    Commented Nov 2, 2015 at 14:15
5
\$\begingroup\$

You could optimize the repetitions with declaring a variable for the count of numbers the user should insert. That also eliminates the "magic number" 3. I called it numberCount. Then you can optimize with loops. that also opens you the option to change the number very quickly, because you only need to change the number once in your code.

So the code turns into this:

static const int numberCount = 3;

static int guess[] = new int[numberCount],
           actual[]= new int[numberCount];

private static void aquireGuess()
{
    Scanner keyboard = new Scanner(System.in);
    System.out.println("Time to play the lottery please choose " + numberCount + " numbers ranging from zero to nine.");

    for(int index = 1; index <= numberCount + 1; index++)
    {
        System.out.print("Please enter number " + index + ":");
        guess[numberCount] = keyboard.nextInt();
    }
    keyboard.close();
}

private static void generateLottery()
{
    for(int index = 1; index <= numberCount + 1; index++)
    {
        actual[index] = (int)(Math.random()*10);
    }
}

private static bool isInRightOrder()
{
    for(int index = 0; index <= numberCount + 1; index++)
    {
        if(guess[index] != actual[index])
            return false;
    }
    return true;
}

private static void compare()
{   
    if(isInRightOrder())
    {
        System.out.println("Congradulations " numberCount + " matches in exact order!!! You win $1,000,000.");
        return;
    }
    ...
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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