0
\$\begingroup\$

This code was done to allow a user to input, edit, search, delete and print a report of all the products in the company. Is there any way to improve my code?

public class Admin
    /**
     * @param args the command line arguments
     */
    public static void main(String[] args) {

        String menuTask, exit; 

        Scanner sc = new Scanner(System.in); // Allows user input

        Products product = new Products(); // Creates the Products class

        System.out.println("BRIGHT FUTURE TECHNOLOGIES APPLICATION");
        System.out.println("***************************************");
        System.out.println("Enter (1) to launch menu or any other key to exit: ");
        exit = sc.next();
        if (exit.equals("1") == false) {
            product.ExitApplication(); // Exits the application 
        }

        do {
            menuTask = product.DisplayMenu(); // Displays the menu
            switch (menuTask) {
                case "1":
                    exit = product.CaptureProduct(); // Allows user to capture a product
                    break;
                case "2":
                    exit = product.SearchProduct(); // Allows user to search for a product
                    break;
                case "3":
                    exit = product.UpdateProduct(); // Allows user to update the product data
                    break;
                case "4":
                    exit = product.DeleteProduct(); // Allows user to delete a product
                    break;
                case "5":
                    exit = product.DisplayReport(); // Displays all the products and its data
                    break;
                case "6":
                    product.ExitApplication(); // Exits the application
                    break;
                default:
                    System.out.println("Thats not a valid key"); // Displays if the user enters an invalid key
            }
        } while (exit.equals("1")); // Loops executes at least once and will continue until the user wants to exit

    }

}

Products.java

public class Products {

    Scanner sc = new Scanner(System.in);

    ReportData data = new ReportData();

    public String SearchProduct() {
 
        int locationOfProduct;
        
        System.out.println("Please enter the product code to search: ");
        locationOfProduct = data.getLocationOfProduct(sc.next());
        System.out.println("***************************************");
        
        if (locationOfProduct != -1) {
            System.out.println("***************************************");
            System.out.println("PRODUCT SEARCH RESULTS");
            System.out.println("***************************************");
            data.toString(locationOfProduct);
        } else {
            
            System.out.println("The product cannot be located. Invalid product key");
            
        }
        
        System.out.println("Enter (1) to launch the menu or any other key to exit");
        
        return sc.next();
    }

    public void SaveProduct(String prodName, String prodCat, String prodCode, String prodSup, String prodWar, int prodLevel, double prodPrice) {
        data.setProductName(prodName);
        switch (prodCat) {
            case "1":
                data.setProductCat("Desktop Computer");
                break;
            case "2":
                data.setProductCat("Laptop");
                break;
            case "3":
                data.setProductCat("Tablet");
                break;
            case "4":
                data.setProductCat("Printer");
                break;
            case "5":
                data.setProductCat("Gaming Console");
                break;
        }
        data.setProductCode(prodCode);
        data.setProductSup(prodSup);
        data.setProductWar(prodWar);
        data.setProductLevel(prodLevel);
        data.setProductPrice(prodPrice);

    }

    public String UpdateProduct() {
        
        String update, war;
        int locationOfProduct;

        System.out.println("Please enter the product code to update: ");
        locationOfProduct = data.getLocationOfProduct(sc.next());
        System.out.println("***************************************");

        if (locationOfProduct != -1) {
            System.out.println("Update the warranty? (y)  Yes, (n) No ");
            update = sc.next();

            if (update.equals("y")) {

                System.out.printf("Indicate the new warrenty for %s. Enter (1) for 6 months or any other key for 2 years ", data.getProductName().get(locationOfProduct));
                if (sc.next().equals("1")) {
                    war = "6 Months";
                } else {
                    war = "2 years";
                }

                data.getProductWar().set(locationOfProduct, war);

                System.out.println("Update the product price? (y) Yes, (n) No ");
                update = sc.next();

                if (update.equals("y")) {

                    System.out.printf("Enter the new price for %s ", data.getProductName().get(locationOfProduct));
                    data.getProductPrice().set(locationOfProduct, sc.nextDouble());

                }

                System.out.println("Update the stock level? (y) Yes, (n) No ");
                update = sc.next();

                if (update.equals("y")) {
                    System.out.printf("Enter the new stock level for %s ", data.getProductName().get(locationOfProduct));
                    data.getProductLevel().set(locationOfProduct, sc.nextInt());
                }

            }

        } else {
            System.out.println("The product cannot be located. Invalid product key");
        }

        System.out.println("Product details have been saved successfully");
        System.out.println("Enter (1) to launch the menu or any other key to exit");
        return sc.next();

    }

    public String DeleteProduct() {
        int locationOfProduct;

        System.out.println("Please enter the product code to delete: ");
        System.out.println("***************************************");

        locationOfProduct = data.getLocationOfProduct(sc.next());

        if (locationOfProduct != -1) {
            data.getProductCat().remove(locationOfProduct);
            data.getProductCode().remove(locationOfProduct);
            data.getProductLevel().remove(locationOfProduct);
            data.getProductName().remove(locationOfProduct);
            data.getProductPrice().remove(locationOfProduct);
            data.getProductSup().remove(locationOfProduct);
            data.getProductWar().remove(locationOfProduct);
        } else {

            System.out.println("The product cannot be located. Invalid product key");

        }

        System.out.println("Enter (1) to launch the menu or any other key to exit");

        return sc.next();

    }

    public String DisplayMenu() {
        System.out.println("Please select one of the following menu items: ");
        System.out.println("(1) Capture a new product. \n" + "(2) Search for a product. \n" + "(3) Update a product. \n" + "(4) Delete a product. \n" + "(5) Print report. \n" + "(6) Exit application. ");
        return sc.next();
    }

    public String CaptureProduct() {
        String name, cat, sup, war;
        int level;
        double price;

        System.out.println("CAPTURE A NEW PRODUCT");
        System.out.println("*********************");

        System.out.println("Enter the product code: ");
        String code = sc.nextLine();

        System.out.println("Enter the product name: ");
        name = sc.nextLine();

        System.out.print("Select the product Category:\n" + "Desktop Computer - 1\n" + "Laptop - 2\n" + "Tablet - 3\n" + "Printer - 4\n" + "Gaming Console - 5 \nProduct Category >> ");
        cat = sc.nextLine();

        System.out.println("Indicate the product warrenty. Enter (1) for 6 months or any other key for 2 years");
        if (sc.next().equals("1")) {
            war = "6 Months";
        } else {
            war = "2 years";
        }

        System.out.printf("Enter the price for %s: ", name);
        price = sc.nextDouble();

        System.out.printf("Enter the stock level for %s: ", name);
        level = sc.nextInt();

        System.out.printf("Enter the supplier for %s: ", name);
        sup = sc.next();

        SaveProduct(name, cat, code, sup, war, level, price); // Saves the details of the product

        System.out.println("Product details have been saved successfully");
        System.out.println("Enter (1) to launch the menu or any other key to exit");
        return sc.next();
    }

    public void ExitApplication() {
        System.exit(0);
    }

    public String DisplayReport() {

        double totalPrice = 0, itemPrice;
        int count;

        System.out.println("PRODUCT REPORT");
        System.out.println("=====================================================");
        count = data.getProductCat().size();

        for (int i = 0; i < count; i++) {

            System.out.printf("PRODUCT %s\n", i + 1);
            System.out.println("-----------------------------------------------------");
            data.toString(i);
            System.out.println("-----------------------------------------------------");
            itemPrice = Integer.parseInt(data.getProductSup().get(i)) * data.getProductPrice().get(i);
            totalPrice = totalPrice + itemPrice ;
            
        }

        System.out.println("=====================================================");
        System.out.printf("TOTAL PRODUCT COUNT: \t%s\n", count);
        System.out.printf("TOTAL PRODUCT VALUE: \tR %s\n", totalPrice);
        System.out.printf("AVERAGE PRODUCT VALUE: \tR %s\n", totalPrice / count);

        System.out.println("=====================================================");
        System.out.println("Enter (1) to launch the menu or any other key to exit");
        return sc.next();
    }
}

ReportData.java

public class ReportData {

    ArrayList<String> productName = new ArrayList<>();
    ArrayList<String> productCat = new ArrayList<>();
    ArrayList<String> productCode = new ArrayList<>();
    ArrayList<String> productSup = new ArrayList<>();
    ArrayList<String> productWar = new ArrayList<>();
    ArrayList<Integer> productLevel = new ArrayList<>();
    ArrayList<Double> productPrice = new ArrayList<>();

    public void setProductName(String productName) {
        this.productName.add(productName);
    }

    public void setProductCat(String productCat) {
        this.productCat.add(productCat);
    }

    public void setProductCode(String productCode) {
        this.productCode.add(productCode);
    }

    public void setProductSup(String productSup) {
        this.productSup.add(productSup);
    }

    public void setProductWar(String productWar) {
        this.productWar.add(productWar);
    }

    public void setProductLevel(int productLevel) {
        this.productLevel.add(productLevel);
    }

    public void setProductPrice(double productPrice) {
        this.productPrice.add(productPrice);
    }

    public ArrayList<String> getProductName() {
        return productName;
    }

    public ArrayList<String> getProductCat() {
        return productCat;
    }

    public ArrayList<String> getProductCode() {
        return productCode;
    }

    public ArrayList<String> getProductSup() {
        return productSup;
    }

    public ArrayList<String> getProductWar() {
        return productWar;
    }

    public ArrayList<Integer> getProductLevel() {
        return productLevel;
    }

    public ArrayList<Double> getProductPrice() {
        return productPrice;
    }
    
    public int getLocationOfProduct(String prodCode) {
        
        return getProductCode().indexOf(prodCode); // Gets the location of the product in the array
       
    }
    
    public void toString(int locationOfProduct){
            System.out.printf("PRODUCT CODE:\t\t%s\n", getProductCode().get(locationOfProduct));
            System.out.printf("PRODUCT NAME:\t\t%s\n", getProductName().get(locationOfProduct));
            System.out.printf("PRODUCT WARRANTY:\t%s\n", getProductWar().get(locationOfProduct));
            System.out.printf("PRODUCT CATEGORY:\t%s\n", getProductCat().get(locationOfProduct));
            System.out.printf("PRODUCT PRICE:\t\tR%s\n", getProductPrice().get(locationOfProduct));
            System.out.printf("PRODUCT STOCK LEVEL:\t%s\n", getProductLevel().get(locationOfProduct));
            System.out.printf("PRODUCT SUPPLIER:\t%s\n", getProductSup().get(locationOfProduct));
    }

    

}
\$\endgroup\$
3
  • \$\begingroup\$ Uhh... is your company ok with you posting the source of an internal tool online? It seems like a generic tool, nothing too closely business-related, but you still might get in trouble, especially if tyou're working for an IT company. \$\endgroup\$ Sep 7, 2021 at 8:32
  • \$\begingroup\$ I was going to ask if this was actually used by some company to run their business or was this a school assignment? Reviews are wholly different for production code and beginner level class room code. \$\endgroup\$ Sep 7, 2021 at 10:31
  • \$\begingroup\$ It's an assignment I'm working on. \$\endgroup\$
    – Umsugi
    Sep 7, 2021 at 16:01

2 Answers 2

4
\$\begingroup\$

Naming Conventions

Methods in Java use lowerCamelCase, as seen in your ReportData class. PascalCase ist only used for classes, so the method names in the Products class should be changed

Confusing names

  • Products isn't a class related to products, it handles user input (suggesting Menu instead)
  • exit isn't an exit flag, it stores the user's menu selection (suggesting input instead)
  • setProduct*() doesn't set anything, it adds to a list (suggesting addProduct*() instead)
  • getProduct*() doesn't get a single product's value, it gets a list of values of all products (suggesting getAllProducts*() instead)
  • ReportData.toString() prints a product composed from a list. As this overloads Object.toString() this is especially problematic, since methods with that name are expected to return a string representation of the entire ReportData object. Suggesting printAll() instead.

System.exit()

In general, this method should be used rarely. It stops the JVM immediately, and so the rest of the program may not be able to clean up and/or terminate gracefully. This is not a problem here, but if you want to save your data to a file, this won't be possible unless you use a shutdown hook, introducing unneeded complexity.

Also, it's not necessary in this case. Instead, exit may be set to anything but "1" and the do-while-loop terminates normally, as if the user requested it in one of the other dialogues.

Comparing to boolean

exit.equals("1") == false is clunky to read. equals() already returns a boolean that can be used directly in the if-statement, so no need to compare it to false.

if (exit.equals("1") == false)
// reads "if exit equals "1" is the same as false" --> weird and long

if (!exit.equals("1"))
// reads "if not exit equals "1"" --> better and shorter

ReportData class

It seems like there is an array list for each of the product's attributes (name, code, warranty), and a product is everything at one index. This is overcomplicated and will lead to bugs and inconsistent data eventually.

Instead make a class Product where each instance has one name/code/etc. You can then manipulate a simpler and consistent ArrayList<Product> instead. This list could even be a static member of the Product class. After all, this is what OOP is about: model a real-life product in a simple/abstract way, a Product class for example.

Input validation

In most cases, the program does simple string comparisons which seems stable enough. At some point, the user has to input a number (int/float) though which will cause InputMismatchExceptions if something invalid is entered. These aren't caught here. Also, the numbers should be checked according to common sense and/or business logic, entering a negative prize is nonsensical at best and expensive at worst.

Menu implementation

The program asks in a pre-menu for a "1" to launch the menu or something else to quit and then offers a seperate quit option in the main menu. This might be confusing or annoying to use. If the pre-menu is removed, the do-while loop in main could print the main menu each time it loops while also providing the functionality of exiting the program. This way the methods in Products wouldn't need to print the pre-menu themselves (which is an unnecessary duplication of code that wastes time if the dialogue is changed) and the exit flag can be removed from most cases. It'll still be needed in the exiting case, since the do-while loop has to end somehow.

The numerical menu choices could be constants or an enum.

In big menus it can make sense to move each dialogue to its own class and then have these classes either extend an abstract base class or implement an interface. I'm keeping this short and vague since in your case, having one class handle the entire interface is still reasonable enough.

Long println

System.out.println("(1) Capture a new product. \n" + "(2) Search for a product. \n" + "(3) Update a product. \n" + "(4) Delete a product. \n" + "(5) Print report. \n" + "(6) Exit application. ");

This line is weird to read and should be split onto multile println()s.

\$\endgroup\$
0
\$\begingroup\$

One additional comment:

You text-based menus are not a professional way of creating a user interface (it feels like in the 1980s). On your learning curve, you probably haven't covered graphical user interfaces (e.g. using Swing or a web server) yet, so that's fine. But maybe you want to read on how to improve your professional skills.

Text-based programs have their professional use mainly when they are controlled by command-line parameters, meaning to get all input from the args in main(String[] args). Then they can be easily automated in batch files or shell scripts. Interactive programs typically have a GUI or a web front end.

Talking about user input/output: professional code separates data processing from the user interface, having one set of classes responsible for processing (maintaining a products list, searching, calculating cost etc., not talking with the user), and different classes for input and output (entering descriptions of products, querying the list, accepting orders, etc.).

Well-designed code even allows to easily switch between GUI and web front-end, by just using a different set of user interface classes, and using the same processing classes.

\$\endgroup\$

Your Answer

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

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