Home > Mobile >  Why does this method print both messages whether update successful or not?
Why does this method print both messages whether update successful or not?

Time:11-05

I have a program to update vehicle inventory. I call the updateVehicle()...it should loop through the arrayList of vehicles to look for a match based on what the user input. In the if statement, if a match is found, update the vehicle in the arrayList with what the user input, display the updated details, display a successfully updated message. If a match was not found, just display not found message. The code works and will update the vehicle with the correct message if there is only one vehicle in the arrayList. However, if there is more than one vehicle in the arrayList, it will update it, but still prints both messages.

public void updateVehicle(String makeCurrent, String modelCurrent, String colorCurrent, int yearCurrent, int mileageCurrent,
            String makeUpdated, String modelUpdated, String colorUpdated, int yearUpdated, int mileageUpdated) {

        for (int i = 0; i < listOfVehicles.size(); i  ) {
            AutoInv vehicle = listOfVehicles.get(i);
            if (vehicle.getMake().equalsIgnoreCase(makeCurrent) && vehicle.getModel().equalsIgnoreCase(modelCurrent)
                    && vehicle.getColor().equalsIgnoreCase(colorCurrent) && vehicle.getYear() == yearCurrent
                    && vehicle.getMileage() == mileageCurrent) {
                vehicle.setMake(makeUpdated);
                vehicle.setModel(modelUpdated);
                vehicle.setColor(colorUpdated);
                vehicle.setYear(yearUpdated);
                vehicle.setMileage(mileageUpdated);
                System.out.println("\nVehicle updated successfully!\n");
                displayCurrentVehicleEntry();
//                  break;
            }
            else {
                System.out.println("\nVehicle not found in inventory!");
            }

        }
    }

CodePudding user response:

The problem is that not all vehicles will match the condition. Yet you print out the not found message for each one not found. Use a boolean to determine if at least one vehicle was found (I presume there could be more than one). As soon as it is found, set the boolean to true. Then use that to determine if the error message should be printed.

 public void updateVehicle(String makeCurrent, String modelCurrent,
         String colorCurrent, int yearCurrent, int mileageCurrent,
         String makeUpdated, String modelUpdated, String colorUpdated,
         int yearUpdated, int mileageUpdated) {

     boolean found = false;
     for (int i = 0; i < listOfVehicles.size(); i  ) {
         AutoInv vehicle = listOfVehicles.get(i);
         if (vehicle.getMake().equalsIgnoreCase(makeCurrent)
                 && vehicle.getModel().equalsIgnoreCase(modelCurrent)
                 && vehicle.getColor().equalsIgnoreCase(colorCurrent)
                 && vehicle.getYear() == yearCurrent
                 && vehicle.getMileage() == mileageCurrent) {
             vehicle.setMake(makeUpdated);
             vehicle.setModel(modelUpdated);
             vehicle.setColor(colorUpdated);
             vehicle.setYear(yearUpdated);
             vehicle.setMileage(mileageUpdated);
             System.out.println("\nVehicle updated successfully!\n");
             displayCurrentVehicleEntry();
             found = true; // at least one was found.
         }
     }
     if (!found) {
          System.out.println("\nVehicle not found in inventory!");
     }
 }

Note: If you know for certain that only one vehicle will be found per query, then you can skip the boolean and just do a return statement after the update. If the loop finishes without returning, then no vehicle was found. So just put the print statement after the loop block. It will only print if the loop doesn't find anything.

CodePudding user response:

Couple of things I would suggest you can do (I know, I know, there are thousands different ways of coding this).

  1. Instead of passing 10 arguments in updateVehicle, pass 2 objects of type AutoInv, 1 for current and another for updated
  2. Instead of comparing all properties of AutoInv 1 by 1, rely on the AutoInv.equals method (write it in your pojo class if you don't have it already)
  3. Use (if possible) lambda expressions to search the current vehicle in your list.

POJO Class

public class Vehicle {
    
        private String make;
        private String model;
        private String color;
        private int year;
        private int mileage;
        
        public Vehicle(String make, String model, String color, int year, int mileage) {
            super();
            this.make = make;
            this.model = model;
            this.color = color;
            this.year = year;
            this.mileage = mileage;
        }
        
        //Getters and setters
    
        //hashcode method

        //toString method
    
        @Override
        public boolean equals(Object obj) {
            if (this == obj) {
                return true;
            }
            if (!(obj instanceof Vehicle)) {
                return false;
            }
            Vehicle other = (Vehicle) obj;
            return Objects.equals(color, other.color) && Objects.equals(make, other.make) && mileage == other.mileage
                    && Objects.equals(model, other.model) && year == other.year;
        }
    
    }

Class with search/update logic

public class Main {

    ArrayList<Vehicle> listOfVehicles = new ArrayList<>();
    
    Main(){
        listOfVehicles.add(new Vehicle("Honda", "Accord", "Black", 2020, 15000));
        listOfVehicles.add(new Vehicle("Ford", "F-150", "White", 2021, 11000));
        listOfVehicles.add(new Vehicle("Chevrolet", "Camaro", "Red", 2018, 32500));
    }
    
     public void updateVehicle(Vehicle current, Vehicle updated) {

         Optional<Vehicle> found = listOfVehicles.stream().filter(vehicle -> vehicle.equals(current)).findFirst();

         if (found.isPresent()) {
             System.out.print("Found: ");
             System.out.println(found.get());
             found.get().setColor(updated.getColor());
             found.get().setMake(updated.getMake());
             found.get().setMileage(updated.getMileage());
             found.get().setModel(updated.getModel());
             found.get().setYear(updated.getYear());
             System.out.println("\nVehicle updated successfully!\n");
         } else {
             System.out.println("\nVehicle not found in inventory!");
         }

     }
     
     public static void main(String[] args) {
        Main main = new Main();
        System.out.println("Original list");
        System.out.println(main.listOfVehicles);
        
        Vehicle toFind = new Vehicle("Chevrolet", "Camaro", "Red", 2018, 32500);
        Vehicle updated = new Vehicle("Chevrolet", "Camaro", "Black", 2018, 45000);

        main.updateVehicle(toFind, updated);
        
        System.out.println("Modified list");
        System.out.println(main.listOfVehicles);
        
    }
     
}

Console Output

Original list
[Vehicle [make=Honda, model=Accord, color=Black, year=2020, mileage=15000], Vehicle [make=Ford, model=F-150, color=White, year=2021, mileage=11000], Vehicle [make=Chevrolet, model=Camaro, color=Red, year=2018, mileage=32500]]
Found: Vehicle [make=Chevrolet, model=Camaro, color=Red, year=2018, mileage=32500]

Vehicle updated successfully!

Modified list
[Vehicle [make=Honda, model=Accord, color=Black, year=2020, mileage=15000], Vehicle [make=Ford, model=F-150, color=White, year=2021, mileage=11000], Vehicle [make=Chevrolet, model=Camaro, color=Black, year=2018, mileage=45000]]

I hope this is helpful,

Happy coding!

Karl

  •  Tags:  
  • java
  • Related