Vending machine implementation

How would one optimize this code?

import java.util.*;
public class VendingMachineAssignement {

    public static void main(String[] args) {
        Scanner keyboard = new Scanner(System.in);

        double item1 = 1.25;
        double item2 = .75;
        double item3 = .90;
        double item4 = .75;
        double item5 = 1.50;
        double item6 = .75;

        System.out.print("Enter an item number: ");
        int item = keyboard.nextInt();

        System.out.print("Enter the amount paid: ");
        double paid = keyboard.nextDouble();

        if (item == 2 || item == 4 || item == 6)
        {
            if (paid >= item2)
            {
                System.out.println("Thank you for buying item " + item + ", your change is $" + (paid-item2) + ". Please come again!");
            }
            if (paid = item1) 
            {
                System.out.println("Thank you for buying item " + item + ", your change is $" + (paid-item1) + ". Please come again!");
            }
            if (paid = item3) 
            {
                System.out.println("Thank you for buying item " + item + ", your change is $" + (paid-item3) + ". Please come again!");
            }
            if (paid = item5) 
            {
                System.out.println("Thank you for buying item " + item + ", your change is $" + (paid-item5) + ". Please come again!");
            }
            if (paid < item5)
            {
                System.out.println("Please insert another " + "$" + (item5-paid));
            }
        }

    }

}

And also, for example, if I enter 5 for item and .10 for paid, then why does it say Please insert another $1.4 instead of 1.40? Even though 1.50 - 1.4 and 1.50 - 1.40 would be the same, how come it does not display 1.40?

Things I would fix:

All of your condition test statements do exactly the same thing.

else if (item == 1)
{
    if (paid >= item1) 
    {
        System.out.println("Thank you for buying item " + item + ", your change is $" + (paid-item1) + ". Please come again!");
    }
    if (paid = item3) 
    {
        System.out.println("Thank you for buying item " + item + ", your change is $" + (paid-item3) + ". Please come again!");
    }
    if (paid < item3)
    {
        System.out.println("Please insert another " + "$" + (item3-paid));
    }
}
...

You can use arrays to help solve this issue.

double[] prices = new double[] { 1.25, 0.75, 0.9, 0.75, 1.5, 0.75 };

Then you can access those specific values with prices[item - 1]. Remember that arrays start with the index 0, so without subtracting 1 you will end up with an off-by-one error. The comparison is now simplified.

if (paid >= (prices[item - 1]))
{
    System.out.println("Thank you for buying item " + item + ", your change is " + (paid - prices[item - 1]) + ". Please come again!");
}

You ask the user to insert more money, but never actually allow him to.

if (paid < item2)
{
    System.out.println("Please insert another " + "$" + (item2-paid));
}

This can easily be solved with a while loop and another pair of input request statements.

You never close your Scanner when you are finished with it (a memory issue).

keyboard.close();

You never check if the item you buy is actually within the bounds of the array length.

if (!(item <= prices.length))
{
    System.out.println("Sorry, we don't have that item in stock.");
    keyboard.close();
    return;
}

When working with money, it is hard to work with doubles, they are too imprecise. An example of this is that 0.42 - .21 is likely something like 0.209999999999999997 rather than 0.21 exactly.

We are going to use the NumberFormat class to make sure that the our double values will stay accurate.

NumberFormat.getCurrencyInstance().format(number)

Recommendations that are optional:

Don't do this:

import java.util.*;

It could be problematic for the compiler to import a bunch of packages at once. If two packages provide the same type, both are imported, and the type is used in the class, a compile-time error occurs. This is described in JLS 6.5.5.1:


Otherwise, if a type of that name is declared by more than one type-import-on-demand declaration of the compilation unit, then the name is ambiguous as a type name; a compile-time error occurs.

In addition, it also saves a bit of memory. And your IDE (if you use one), should have the ability to do this automatically.

You might note that this:

public static void main(String... args)

Is a bit different than your usual:

public static void main(String[] args)

They both do the same thing, but one uses variable arity parameters.

The Scanner.nextInt() method does not read the last newline character of your input, and thus a newline could be consumed if you call Scanner.nextLine. I prefer to use Scanner.nextLine since you can input any valid String, and then just parse it into the type you need (as long as it's a valid type to parse it to).

Final code:

import java.text.NumberFormat;
import java.util.Scanner;

public class Test
{
    public static void main(String... args)
    {
        Scanner keyboard = new Scanner(System.in);
        double[] prices = new double[] { 1.25, 0.75, 0.9, 0.75, 1.5, 0.75 };
        int item = 0;

        System.out.print("Enter an item number: ");
        item = Integer.parseInt(keyboard.nextLine());
        if (!(item <= prices.length))
        {
            System.out.println("Sorry, we don't have that item in stock.");
            keyboard.close();
            return;
        }

        System.out.print("Enter the amount paid: ");
        double paid = Double.parseDouble(keyboard.nextLine());

        while (paid < (prices[item - 1]))
        {
            System.out.print("You need " + NumberFormat.getCurrencyInstance().format(Math.abs((paid - prices[item - 1]))) + " more for item " + item);
            paid += Double.parseDouble(keyboard.nextLine());
        }
        System.out.println("Thank you for buying item " + item + ", your change is " + NumberFormat.getCurrencyInstance().format((paid - prices[item - 1])) + ". Please come again!");

        keyboard.close();
    }
}

StackExchange Code Review Q#37376, answer score: 21