HND Java module (vending machine simulator)

Question

As part of the distinction requirement, I am required to ask for testing/review of my code. There was no time allotted for this in class, and as such I am turning to the folks of StackExchange for help! This is a great way of doing it for me, as I can directly reference it in my bibliography, and easily format the critique to fit alongside my report for the project.

The line from the assignment brief reads thusly: "Perform user and expert testing of your developed application. Provide evidence of your discussion with users and experts. How their feedback has affected your application."

I would appreciate any time you could spare for this, and would kindly request that you look upon the following areas:

• Code layout
• Ease of use
• Possible improvements (including where inheritance/polymorphism could have been used)
• Critical errors

I would like to preface that I am no master coder, and with my embarrassment out of the way, I present the Java code (zip file can be found here):

Main

package vendingMachine;

public class Main {

    public static void main (String[] args) {

        VendMachine.main(args);

    }

}    

VendMachine class

package vendingMachine;

import java.util.Scanner;   // Imports scanner functionality for user input

public class VendMachine {

    public static void main(String[] args) {



        /* ----- Initialising class for handling moneys and transaction calculations ------ */

        TransactionProcess transaction = new TransactionProcess();   

        /* ------------------------------------------------------------------------------- */




        /* -- Creating a class for each item of stock, and then setting their values using 
         the appropriate method --------------------------------------------------------- */


        Item crispsOne = new Item();
        crispsOne.setStock("Cheese & Onion crisps", 10, "A1", 0.70);

        Item crispsTwo = new Item();
        crispsTwo.setStock("Salt & Vinegar crisps", 10, "A2", 0.70);

        Item crispsThree = new Item();
        crispsThree.setStock("Ready Salted crisps", 10, "A3", 0.70);

        Item bottleOne = new Item();
        bottleOne.setStock("Coke", 10, "B1", 1.10);

        Item bottleTwo = new Item();
        bottleTwo.setStock("Dr Pepper", 10, "B2", 1.10); 

        Item bottleThree = new Item();
        bottleThree.setStock("Lilt", 10, "B3", 1.10);

        Item chocOne = new Item();
        chocOne.setStock("Lion Bar", 10, "C1", 0.55);

        Item chocTwo = new Item();
        chocTwo.setStock("Star Bar", 10, "C2", 0.55);

        Item chocThree = new Item();
        chocThree.setStock("Yorkie", 10, "C3", 0.60);

        /* ------------------------------------------------------------------------------- */



        /* - Creating an interface class, and then using it's methods to prompt the user - */

        UserInterface mainInterface = new UserInterface();
        mainInterface.getMoney();
        System.out.println("Available credit: " + mainInterface.moneyIn + "\n"); 
        mainInterface.getInput();
        System.out.println("\nYou entered " + mainInterface.selection);

        /* ------------------------------------------------------------------------------- */



        /* -------------- A switch case for each choice of item in the vending machine ------------------ */

        switch (mainInterface.selection) {

        case "A1":

            transaction.itemPrice = crispsOne.price;
            transaction.change = transaction.Transaction(mainInterface.moneyIn, transaction.itemPrice);

            if (transaction.enoughCredit() == true) {
                mainInterface.displayChoice(crispsOne.itemName, crispsOne.price, transaction.change);
                crispsOne.stockAdjust();
                break;
            }
            else {  

                mainInterface.catchMeAnError();
                break;
            }

        case "A2":

            transaction.itemPrice = crispsTwo.price;
            transaction.change = transaction.Transaction(mainInterface.moneyIn, transaction.itemPrice);

            if (transaction.enoughCredit() == true) {
                mainInterface.displayChoice(crispsTwo.itemName, crispsTwo.price, transaction.change);
                crispsTwo.stockAdjust();
                break;
            }
            else {  

                mainInterface.catchMeAnError();
                break;Ve
            }

        case "A3":

            transaction.itemPrice = crispsThree.price;
            transaction.change = transaction.Transaction(mainInterface.moneyIn, transaction.itemPrice);

            if (transaction.enoughCredit() == true) {
                mainInterface.displayChoice(crispsThree.itemName, crispsThree.price, transaction.change);
                crispsThree.stockAdjust();
                break;
            }
            else {  

                mainInterface.catchMeAnError();
                break;
            }

        case "B1":

            transaction.itemPrice = bottleOne.price;
            transaction.change = transaction.Transaction(mainInterface.moneyIn, transaction.itemPrice);

            if (transaction.enoughCredit() == true) {
                mainInterface.displayChoice(bottleOne.itemName, bottleOne.price, transaction.change);
                bottleOne.stockAdjust();
                break;
            }
            else {  

                mainInterface.catchMeAnError();
                break;
            }

        case "B2":

            transaction.itemPrice = bottleTwo.price;
            /*      sets itemPrice in the object transaction to the price stored in the object bottleTwo     */

            transaction.change = transaction.Transaction(mainInterface.moneyIn, transaction.itemPrice); 
            /* sets change variable in transaction object equal to moneyIn - itemPrice using the Transaction method within
             * this instantiation of the TrasnactionProcess class, so we know what change the customer may need     */

            if (transaction.enoughCredit() == true) {
            /*  Checks whether or not enough money has been entered for this selection using a method of the TransactionProcess Class  */

                mainInterface.displayChoice(bottleTwo.itemName, bottleTwo.price, transaction.change);
            /* Uses a method of the UserInterface class to display the relevant information to the user, as well as 
             * getting the amount of change needed from the transaction object and tendering said change         */ 

                bottleTwo.stockAdjust();
            /*  Calls method belonging to the Item class for adjusting of stock  */

                break;
            /* Breaks out of the switch case */

            }
            else {                      

                mainInterface.catchMeAnError();
            /* Calls the function to check for an error within the UserInterface class */   
                break;
            /* Breaks out of the switch case */
            }

        case "B3":          

            transaction.itemPrice = bottleThree.price;
            transaction.change = transaction.Transaction(mainInterface.moneyIn, transaction.itemPrice);

            if (transaction.enoughCredit() == true) {
                mainInterface.displayChoice(bottleThree.itemName, bottleThree.price, transaction.change);
                bottleThree.stockAdjust();
                break;
            }
            else {  

                mainInterface.catchMeAnError();
                break;
            }

        case "C1":          

            transaction.itemPrice = chocOne.price;
            transaction.change = transaction.Transaction(mainInterface.moneyIn, transaction.itemPrice);

            if (transaction.enoughCredit() == true) {
                mainInterface.displayChoice(chocOne.itemName, chocOne.price, transaction.change);
                chocOne.stockAdjust();
                break;
            }
            else {  

                mainInterface.catchMeAnError();
                break;
            }


        case "C2":          

            transaction.itemPrice = chocTwo.price;
            transaction.change = transaction.Transaction(mainInterface.moneyIn, transaction.itemPrice);

            if (transaction.enoughCredit() == true) {
                mainInterface.displayChoice(chocTwo.itemName, chocTwo.price, transaction.change);
                chocTwo.stockAdjust();
                break;
            }
            else {  

                mainInterface.catchMeAnError();
                break;
            }

        case "C3":          

            transaction.itemPrice = chocThree.price;
            transaction.change = transaction.Transaction(mainInterface.moneyIn, transaction.itemPrice);

            if (transaction.enoughCredit() == true) {
                mainInterface.displayChoice(chocThree.itemName, chocThree.price, transaction.change);
                chocThree.stockAdjust();
                break;
            }
            else {  

                mainInterface.catchMeAnError();
                break;
            }

        default:

            System.out.println("You made an invalid selection, please try again \n");
            System.out.println("Returning money \n" + "...\n" + "...\n" + "Please collect your " + mainInterface.moneyIn + " change\n");
            break;

        }


        /* ------------------------------------------------------------------------------------------------------------- */     

        int doNotClose;
        Scanner stayOpen = new Scanner(System.in);  // Scanner to keep console window from closing
        doNotClose = stayOpen.nextInt();

    }

}

UserInterface class

package vendingMachine;

import java.util.Scanner;       // scanner functionality imported to get input from user

public class UserInterface {

    double moneyIn;
    double moneyOut;            // Values that need to be stored in the UserInterface class
    boolean isAvailable;
    String selection;

    void getMoney() {

        Scanner getMoney = new Scanner(System.in);
        System.out.println("Please insert money: ");        // Method that prompts user to enter money, and then stores money
        moneyIn = getMoney.nextDouble();
        System.out.println(" ");

    }

    void getInput() {                                       // Method that displays selections and gets and stores choice from user

        Scanner getSelection = new Scanner(System.in);

        System.out.println("Please enter A selection from one of the follwing\n");
        System.out.println("***** Crisps *****");
        System.out.println("[A1] Cheese & Onion, [A2] Salt & Vinegar, [A3] Ready Salted \n");
        System.out.println("***** Bottles of drink *****");             
        System.out.println("[B1] Coke, [B2] Dr Pepper, [B3] Lilt\n");
        System.out.println("***** Chocolate Bars *****");
        System.out.println("[C1] Lion Bar, [C2] Star Bar, [C3] Yorkie\n");

        selection = getSelection.nextLine();
        selection = selection.toUpperCase();
    }

    void displayChoice (String aChoice, double aPrice, double someChange) {    // Method used to display vend process to user

        System.out.println("You chose " + aChoice);
        System.out.println("This costs " + aPrice);
        System.out.println("You are due " + someChange + " change\n ");
        System.out.println("Vending Item \n" + "...\n" + "...\n" + "Please Take your " + aChoice + "\n");
        System.out.println("Vending Change \n" + "...\n" + "...\n" + "Please Take your " + someChange + " change\n");
        System.out.println("Thank you \n");

    }

    void insufficientFunds() {

        // Method to let user know they did not enter enough money, and return this money       
        throw new UnsupportedOperationException();

    }

    void catchMeAnError () {
        try {               
            this.insufficientFunds();
            /* The code that the try block will try to run, allowing the program to continue if there is a problem */   
            } 
            catch (UnsupportedOperationException e) {
                /* This catch will catch the exception thrown from within the insufficientFunds */                       
                System.out.println("You did not enter enough money, please try again \n");
                System.out.println("Returning money \n" + "...\n" + "...\n" + "Please collect your " + this.moneyIn + " change\n");             
                /* As I still want the code to function as whole, this is the message that will be printed to the console when the error 
                   Is caught, as opposed to the standard and confusing error message */
            }

    }

}

Item Class

package vendingMachine;

public class Item {

    String itemName;
    int stockLevel;
    String selectionID; // Values that need to be stored in the item class
    double price;

    void setStock(String aName, int numberOf, String sID, double aPrice) {

        itemName = aName;
        stockLevel = numberOf; // Method to set values of new instances
        selectionID = sID;
        price = aPrice;

    }

    void stockAdjust() {

        this.stockLevel--;   // Method to decrement stock level after successful vend

    }

}

TransactionProcess class

package vendingMachine;

public class TransactionProcess {

    double itemPrice;               // Values that need to be stored in the TransactionProcess class
    double change;

    double Transaction(double moneyIn, double aPrice) {

        double change;
        change = moneyIn - aPrice;   // Method that gets money entered and item price and works out change needed, if any
                                    // and then returns that value
        return change;

    }

    boolean enoughCredit() {

        if (this.change > 0)    
        return true;                // Method that checks if the user has entered enough money for the selected item 
        else return false;          // and returns true or false based on whether or not there is enough moneys

    }

}

Answer 1

Excessive comments

The comments are really excessive. Comments explaining trivial code with English words are completely pointless, and become noise, hindering readability. All the ones that I've read were unnecessary. Let the code speak for itself. If it doesn't, then reorganize it until it does. See also this related answer, and this related blog post.

Too relaxed access to member variables

Most of your class member variables have no access modifiers, making them too accessible from other classes within the same package. This fails good encapsulation and information hiding. I suggest to make everything private, even private final when possible, and add some getters where needed. Public or even protected member variables are extremely rarely justified.

Inappropriate use of exceptions

The insufficientFunds and catchMeAnError methods are nonsensical. Am exception that is always thrown is not an exception, it's a design error. Rewrite this part using conditions.

UnsupportedOperationException is designed for operations that are unsupported by design, and it's not normal to catch this type of exception. It has no place in your example.

Poor design

A simple example where many things went wrong is the Item class:

• The constructor creates a an instance that's practically useless: it has no name, no id, no price
• The setStock method in fact sets everything, so setStockAndAWholeBunchOfOtherStuff would better reflect what it actually does
• The stockAdjust method doesn't hint what kind of adjustment it will do. It decreases stock by one, but this is not intuitive from the signature alone, one has to read the implementation
• The word "item" in itemName is redundant, duplicating what the class name already says
• Name, id, price never change throughout the program, but they are not protected with the final keyword

A better way to write this class would be:

public class Item {

    private final String id;
    private final String name;
    private final double price;

    private int stock;

    public Item(String id, String name, double price, double initialStock) {
        this.id = id;
        this.name = name;
        this.price = price;
        this.stock = initialStock;
    }

    void decreaseStock() {    
        this.stock--;
    }
}

Many of your other classes are similarly flawed. I suggest to go through them one by one, and ask yourself these (and more) questions:

• What is the main purpose of the class? (If more than one, then split it)
• Does the constructor give an object that's ready to use? (It should.)
• Without reading the implementation of a method, just by reading its signature, can the reader guess what the method will do
• What data should the class hide? What member fields does it have that other classes shouldn't need to know about?
• What data should be allowed to change throughout the lifetime of an object of the class? Anything that doesn't make sense to ever change should be declared final