Critique my code

[Deathslice]

So this an assignment that I just finished right now and I would like get back feedback on what I did well and where I should improve. If you may, please offer better suggestions on how I can improve this program many thanks. Also, if you find any bugs, please message me so that I can fix it.

ComputerInventory class

package computerinventory;

import java.util.Scanner;
import java.util.ArrayList;
import java.util.InputMismatchException;

public class ComputerInventory 
{
    private static Scanner read = new Scanner(System.in);

    public static void main(String[] args) 
    {
        ComputerInventory process = new ComputerInventory();
        ArrayList<Computer> computer = new ArrayList<>();

        int option;
     
        do 
        {
         try 
           {
            System.out.println("1.) Add computers to your inventory.");
            System.out.println("2.) Display your Inventory.");
            System.out.println("3.) Remove Computers from your inventory.");
            System.out.println("4.) Display the total cost of your inventory.");
            System.out.println("5.) Quit the program. ");
            option = read.nextInt();

            switch (option) 
            {
                case 1:
                    process.addComputers(computer);
                    System.out.println("");
                    break;
                case 2:
                    process.displayInventory(computer);
                    System.out.println("");
                    break;
                case 3:
                    process.removeComputer(computer);
                    System.out.println("");
                    break;
                case 4:
                    process.totalCost(computer);
                    System.out.println("");
                    break;
                case 5:
                    System.out.println("\nThank you for using my program.");
                    return;
                default:
                    System.out.println("\nThis choice doesn't exist in the menu.\n");
            }
           }
          catch(InputMismatchException e)
          {
              System.out.flush();
              System.out.print("\nData does not match expected output." );
              System.out.print(" Run the program again.\n\n" );
              read.nextLine();
          }
        } while (true);
    }

    public void addComputers(ArrayList<Computer> computer) 
    {
        String serialNumber;
        double computerSpeed;
        int hardDrive;
        double ram;
        boolean functional;
        double cost;

        read.nextLine();

     try   
     {
         System.out.println("\nEnter a unique serial number for this computer");
         serialNumber = read.nextLine();
         
         if(computer.size() > 0)
         {
             for (Computer computer1 : computer) 
             {
                 while(computer1.getSerialNumber().equals(serialNumber)) 
                 {
                     System.out.println("\nNo duplicates allowed, try agian.");
                     serialNumber = read.nextLine();
                 }
             }
         }
        
        System.out.println("\nHow fast is this computer in Ghz?");
        computerSpeed = read.nextDouble();

        System.out.println("\nHow big is the HardDrive in GigaBytes?");
        hardDrive = read.nextInt();

        System.out.println("\nHow much ram does this computer has. ");
        ram = read.nextDouble();

        System.out.println("\nTrue or false, does this computer work?");
        functional = read.nextBoolean();

        System.out.println("\nHow much does this computer cost? ");
        cost = read.nextDouble();
        read.nextLine();

        Computer com = new Computer(serialNumber, computerSpeed, hardDrive, ram, functional, cost);
        
        com.setTotalCost();

        computer.add(com);
        
        System.out.print("\nYou have a total of " + Computer.getTotalComputerObjects());
        System.out.println(" computer(s) in your inventory.");
      }
      catch(InputMismatchException e)
      {
          System.out.flush();
          System.out.print("\nData does not match expected output." );
          System.out.print(" Run the program again.\n\n" );
          read.nextLine();
      }
    }

    public void displayInventory(ArrayList<Computer> computer) 
    {
        int option;
        boolean isInventory = false;

        if (computer.size() > 0) 
        {
          try
          {
            System.out.println("\n1.) Search the Inventory entirely");
            System.out.println("2.) Search the Inventory by serial number.");
            option = read.nextInt();

            if (option == 1) 
            {
                System.out.println("\nHere is the entire Inventory.");

                for (Computer computer1 : computer) 
                {
                    System.out.println(computer1);
                }
            } 
            else if (option == 2) 
            {
                String computerID;

                read.nextLine();

                System.out.println("\nWhat is the serial number for this computer.");
                computerID = read.nextLine();

                for (Computer computer1 : computer) 
                {
                    if (computer1.getSerialNumber().equals(computerID)) 
                    {
                        isInventory = true;
                        System.out.println("\nThe speed of this computer is " + computer1.getComputerSpeed() + " GHz");
                        System.out.println("The HardDrive capacity is " + computer1.getHardDrive() + " GigaBytes");
                        System.out.println("The RAM capacity is " + computer1.getRam() + " GigaBytes");
                        System.out.println("The status of this computer is " + computer1.isFunctional());
                        System.out.println("The cost of this computer is " + computer1.getCost());
                    }
                }
                
                if(isInventory)
                {
                    System.out.println("\nSearching complete");
                }
                else
                    System.out.println("\nThis computer doesn't exist in the inventory.");
            } 
            else 
            {
                System.out.println("\nInvalid option, returning to the main menu.");
            }
          }
          catch(InputMismatchException e)
          {
            System.out.flush();
            System.out.print("\nData does not match expected output." );
            System.out.print(" Run the program again.\n\n" );
            read.nextLine();
          }
        } 
        else 
        {
            System.out.println("\nYou have no computers in your Inventory.");
        }
    }

    public void totalCost(ArrayList<Computer> computer) 
    {
        double cost = 0;

        for (Computer computer1 : computer) 
        {
            cost += computer1.getTotalCost();
        }

        System.out.println("\nThe current total cost is " + cost);
    }

    public void removeComputer(ArrayList<Computer> computer)
    {
        int option;
        int totalComputerObjects;
        int currentTotalComputerObjects = Computer.getTotalComputerObjects();
        
        if(!computer.isEmpty())
        {
            System.out.println("\n1.) To remove a computer based on its Serial Number.");
            System.out.println("2.) To remove computers based on functionality.");
            option = read.nextInt();
            
         try
         {
             if(option == 1)
            {
                String computerID;
                String response;
                
                read.nextLine();
                
                System.out.println("\nWhat is the serial Number for this computer");
                computerID = read.nextLine();
                
                for(int i=0; i<computer.size(); )
                {
                    if(computer.get(i).getSerialNumber().equals(computerID))
                    {
                        computer.remove(i);
                        Computer.removeTotalComputerObjects();
                        i=0;
                        
                        System.out.print("\nIs there another computer that you wish");
                        System.out.print("to remove? Yes or No\n");
                        response = read.nextLine();
                    
                      if(response.equalsIgnoreCase("Yes") && !computer.isEmpty())
                      {
                        System.out.println("\nWhat is the serial Number for this computer");
                        computerID = read.nextLine();
                      }
                      else if(response.equalsIgnoreCase("No"))
                      {
                        System.out.println("\nReturning to the main menu.");
                        break;
                      }
                      else if(computer.isEmpty())
                      {
                        System.out.println("\nSorry, no more computers left to search.");
                        break;
                      }
                    }
                    else 
                        i++;
                }
                
                if(Computer.getTotalComputerObjects() == 0)
                {
                    System.out.println("\nYou have no more computers left in your inventory.");
                }
                else if(Computer.getTotalComputerObjects() < currentTotalComputerObjects)
                {
                    totalComputerObjects = Computer.getTotalComputerObjects();
                    System.out.println("\nYou now have " + totalComputerObjects + " computer(s) in your inventory.");
                }
                else
                {
                    System.out.print("\nThis computer doesn't exist in your inventory.");
                    System.out.println(" No computers were removed.");
                }
            }
            else if(option == 2)
            {
                boolean notBroken = false;
                int totalBrokenComputers =0;
                
                 for (Computer computer1 : computer) 
                 {
                     if (computer1.isFunctional() == notBroken) 
                     {
                         totalBrokenComputers++;
                     }
                 }
                
                for(int i=0; i<computer.size(); )
                {
                        if(computer.get(i).isFunctional() == notBroken)
                        {    
                          computer.remove(i);
                          Computer.removeTotalComputerObjects();
                          i=0;
                          --totalBrokenComputers;
                          
                          if(totalBrokenComputers == 0)
                          {
                              break;
                          }
                        }
                        else
                            i++;
                }
                
                if(Computer.getTotalComputerObjects() == 0)
                {
                    System.out.println("\nYou have no more computers left in your inventory.");
                }
                else if(Computer.getTotalComputerObjects() < currentTotalComputerObjects)
                {
                    totalComputerObjects = Computer.getTotalComputerObjects();
                    System.out.println("\nYou now have " + totalComputerObjects + " computer(s) in your inventory.");
                }
                else
                {
                    System.out.print("\nThis computer doesn't exist in your inventory.");
                    System.out.println(" No computers were removed.");
                }
            }
            else
                System.out.println("\nInvalid option, returning to the main menu.");
          }
          catch(InputMismatchException e)
          {
            System.out.flush();
            System.out.print("\nData does not match expected output." );
            System.out.print(" Run the program again.\n\n" );
            read.nextLine();
          }
        }
        else
            System.out.println("\nYou have no computers in your inventory.");
    }
}

Computer class

package computerinventory;

public class Computer 
{
    private double computerSpeed;
    private int hardDrive;
    private double ram;
    private boolean functional;
    private double cost;
    private String serialNumber;
    private double totalCost;
    private static int totalComputerObjects;

    public Computer(String serialNumber, double computerSpeed, int hardDrive, double ram, boolean functional, double cost) 
    {
        this.serialNumber = serialNumber;
        this.computerSpeed = computerSpeed;
        this.hardDrive = hardDrive;
        this.ram = ram;
        this.functional = functional;
        this.cost = cost;
        totalComputerObjects++;
    }
    
    public Computer()
    {
        
    }
    
    public static int getTotalComputerObjects() 
    {
        return totalComputerObjects;
    }

    public static void removeTotalComputerObjects() 
    {
        totalComputerObjects--;
    }
 
    public double getTotalCost() 
    {
        return totalCost;
    }
    
    public void setTotalCost() 
    {
        this.totalCost = cost;
    }

    public double getComputerSpeed() 
    {
        return computerSpeed;
    }

    public int getHardDrive() 
    {
        return hardDrive;
    }

    public double getRam() 
    {
        return ram;
    }

    public boolean isFunctional() 
    {
        return functional;
    }
    
    public String getSerialNumber() 
    {
        return serialNumber;
    }

    public void setSerialNumber(String serialNumber) 
    {
        this.serialNumber = serialNumber;
    }
    
    public double getCost() 
    {
        return cost;
    }
    
    public void setComputerSpeed(double computerSpeed) 
    {
        this.computerSpeed = computerSpeed;
    }

    public void setHardDrive(int hardDrive) 
    {
        this.hardDrive = hardDrive;
    }

    public void setRam(double ram) 
    {
        this.ram = ram;
    }

    public void setFunctional(boolean functional) 
    {
        this.functional = functional;
    }

    public void setCost(double cost) 
    {
        this.cost = cost;
    }

    @Override
    public String toString() 
    {
        return "\nThe serial Number for this computer is " + serialNumber + 
         "\nSpeed is " + computerSpeed + " GHz.\n" + "hardDrive is " + hardDrive 
         + " GigaBytes.\n" + "RAM is " + ram + " GigaBytes.\n" + "Status is " + functional
         + "\n" + "The cost of this computer " + cost;
    }
       
}

[Deathslice]

Anybody has anything to say?

[jim829]

Patience! It has only been about 4 hours.

In any event I would have tried to modularize the input. Perhaps writing methods to prompt for ints, doubles, Strings or booleans. The methods might also take a String prompt which would be displayed. Each method could then validate the input before returning it. Another thing would be to eliminate the totalComputerObjects static. I'm not certain you really need it. Just return the size of the ArrayList. But I would have to really examine the code and see how it flows before making any changes. The bottom line is, if it ain't broke, don't fix it. The best thing you can do now is forget about it. Try to examine other source code (look at the API class code to see how some things are done). Then come back after you get more experience. You will certainly notice a difference.

Regards,
Jim

[Deathslice]

alright thanks.

[pbrockway2]

Do lots of testing!

private static Scanner read; // don't initialise yet
 
public static void main(String[] args)
{
    if(args.length > 0)
    {
        read = new Scanner(new File(args[0]));
    }
    else
    {
        read = new Scanner(System.in);
    }
    // etc

or pipe the input at the command line. The point is have lots of tests that you can easily run. The best time for them is once you know what the program behaviour should be but before you write the code.

[JosAH]

Your test for a unique serial number (lines #77 ... #90) is broken; suppose the existing serial numbers are 1, 2, 3, 4, 5 and I enter the numbers 2 (not accepted) and 1 in this order; number 1 is incorrectly accepted. I didn't look any further in the code ...

kind regards,

Jos

[Deathslice]

Well can you be more specific as to what you are entering those serial numbers for? is it to find those computers by serial number, delete the computers by serial numbers or are you adding them to the inventory? if you can, please perform some test on that specific instance and relay that information back to me on your results if you have the test and are willing.