Results 1 to 7 of 7
  1. #1
    Deathslice is offline Member
    Join Date
    Jan 2015
    Location
    Miami, FL
    Posts
    86
    Rep Power
    0

    Default Critique my code

    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
    Java Code:
    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
    Java Code:
    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;
        }
           
    }

  2. #2
    Deathslice is offline Member
    Join Date
    Jan 2015
    Location
    Miami, FL
    Posts
    86
    Rep Power
    0

    Default Re: Critique my code

    Anybody has anything to say?

  3. #3
    jim829 is offline Senior Member
    Join Date
    Jan 2013
    Location
    Northern Virginia, United States
    Posts
    6,226
    Rep Power
    14

    Default Re: Critique my code

    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
    The JavaTM Tutorials | SSCCE | Java Naming Conventions
    Poor planning on your part does not constitute an emergency on my part

  4. #4
    Deathslice is offline Member
    Join Date
    Jan 2015
    Location
    Miami, FL
    Posts
    86
    Rep Power
    0

    Default Re: Critique my code

    alright thanks.

  5. #5
    pbrockway2 is offline Moderator
    Join Date
    Feb 2009
    Location
    New Zealand
    Posts
    4,716
    Rep Power
    18

    Default Re: Critique my code

    Do lots of testing!

    Java Code:
    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.
    Last edited by pbrockway2; 02-08-2015 at 06:55 AM.

  6. #6
    JosAH's Avatar
    JosAH is offline Moderator
    Join Date
    Sep 2008
    Location
    Voorschoten, the Netherlands
    Posts
    14,422
    Blog Entries
    7
    Rep Power
    28

    Default Re: Critique my code

    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
    Build a wall around Donald Trump; I'll pay for it.

  7. #7
    Deathslice is offline Member
    Join Date
    Jan 2015
    Location
    Miami, FL
    Posts
    86
    Rep Power
    0

    Default Re: Critique my code

    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.
    Last edited by Deathslice; 02-08-2015 at 05:24 PM.

Similar Threads

  1. Please Critique My Code
    By kathmandu in forum New To Java
    Replies: 3
    Last Post: 06-28-2013, 10:02 PM
  2. Java Code Critique
    By CoderJava in forum New To Java
    Replies: 4
    Last Post: 06-30-2012, 11:14 PM
  3. Critique my first Java Program!
    By Lucid15 in forum New To Java
    Replies: 3
    Last Post: 01-26-2012, 08:56 AM
  4. Please critique
    By jim01 in forum New To Java
    Replies: 4
    Last Post: 09-24-2010, 03:43 AM
  5. Critique Java Game: Help Me Improve
    By gretty in forum New To Java
    Replies: 1
    Last Post: 07-15-2010, 04:30 AM

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •