Results 1 to 7 of 7
Thread: Critique my code
- 02-08-2015, 03:08 AM #1
Member
- Join Date
- Jan 2015
- Location
- Miami, FL
- Posts
- 86
- Rep Power
- 0
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; } }
- 02-08-2015, 06:19 AM #2
Member
- Join Date
- Jan 2015
- Location
- Miami, FL
- Posts
- 86
- Rep Power
- 0
Re: Critique my code
Anybody has anything to say?
- 02-08-2015, 06:55 AM #3
Senior Member
- Join Date
- Jan 2013
- Location
- Northern Virginia, United States
- Posts
- 6,226
- Rep Power
- 15
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,
JimThe JavaTM Tutorials | SSCCE | Java Naming Conventions
Poor planning on your part does not constitute an emergency on my part
- 02-08-2015, 07:06 AM #4
Member
- Join Date
- Jan 2015
- Location
- Miami, FL
- Posts
- 86
- Rep Power
- 0
Re: Critique my code
alright thanks.
- 02-08-2015, 07:51 AM #5
Moderator
- Join Date
- Feb 2009
- Location
- New Zealand
- Posts
- 4,716
- Rep Power
- 19
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
Last edited by pbrockway2; 02-08-2015 at 07:55 AM.
- 02-08-2015, 11:00 AM #6
- Join Date
- Sep 2008
- Location
- Voorschoten, the Netherlands
- Posts
- 14,422
- Blog Entries
- 7
- Rep Power
- 29
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,
JosBuild a wall around Donald Trump; I'll pay for it.
- 02-08-2015, 06:19 PM #7
Member
- Join Date
- Jan 2015
- Location
- Miami, FL
- Posts
- 86
- Rep Power
- 0
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 06:24 PM.
Similar Threads
-
Please Critique My Code
By kathmandu in forum New To JavaReplies: 3Last Post: 06-28-2013, 11:02 PM -
Java Code Critique
By CoderJava in forum New To JavaReplies: 4Last Post: 07-01-2012, 12:14 AM -
Critique my first Java Program!
By Lucid15 in forum New To JavaReplies: 3Last Post: 01-26-2012, 09:56 AM -
Please critique
By jim01 in forum New To JavaReplies: 4Last Post: 09-24-2010, 04:43 AM -
Critique Java Game: Help Me Improve
By gretty in forum New To JavaReplies: 1Last Post: 07-15-2010, 05:30 AM
Bookmarks