HND Java module (vending machine simulator)

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, "

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

StackExchange Code Review Q#91702, answer score: 3