-2

How can we make an ArrayList in java consistent. Means when I have updated the ArrayList in one function, it is updated. But, after the exit from that function it is again setting it's size to 0.

public static ArrayList<Vehicle> al=new ArrayList<Vehicle>();
    java.util.Iterator<Vehicle> itr= al.iterator();

    @SuppressWarnings("unchecked")
    public boolean addVehicleToSlot(Vehicle vehicle) {
        if((vehicle.slotNo<=0||vehicle.slotNo>40)&&(vehicle.getVehicleType()!="Car"
                ||vehicle.getVehicleType()!="Truck"||vehicle.getVehicleType()!="Two Wheeler")){
            new InvalidSlotException("Slot alraedy allotted");
            return false;
        }
        int iter=0;
        int len=al.size();
        if(len==0){
            al.add(vehicle);
            return true;
        }
        while(iter< len){
            Vehicle veh=(Vehicle)itr.next();
            if(veh.getSlotNo()==vehicle.getSlotNo()){
                new SlotNotFoundException("No slot allotted");
                return false;
            }
            else{
                al.add(vehicle);
                for(Vehicle vehi:al){
                    System.out.println(al);
                }
                return true;
            }
        }
        return false;
    }

My function call is this:

    Vehicle v1=new Vehicle(o1,"ts75","Truck",37,48);
    ParkingManagement p1=new ParkingManagement();
    System.out.println(p1.addVehicleToSlot(v1));

Please help me out with this.

Jens
  • 67,715
  • 15
  • 98
  • 113
  • 1
    Please don't use [raw types](https://stackoverflow.com/questions/2770321/what-is-a-raw-type-and-why-shouldnt-we-use-it). – Lino Jun 25 '18 at 06:08
  • 1
    Also: Questions seeking debugging help ("why isn't this code working?") must include the desired behavior, a specific problem or error and the shortest code necessary to reproduce it in the question itself. Questions without a clear problem statement are not useful to other readers. See: [How to create a Minimal, Complete, and Verifiable example](https://stackoverflow.com/help/mcve). – Lino Jun 25 '18 at 06:08
  • I recommend not to store the slot inside the vehicle object. It just does not belong there... It is a property of the `ParkingManangement`, isn't it? – deHaar Jun 25 '18 at 06:08
  • 1
    This is not the complete code. Make sure that the scope of the arraylist is bigger than this function if you want to see that getting updated – gargkshitiz Jun 25 '18 at 06:09
  • 1
    You don't need an iterator here, I think, if you just want the `addVehicleToSlot` method to simply add a vehicle to a list. – Tim Biegeleisen Jun 25 '18 at 06:09
  • @jwenting It is now, as Jens updated the question to not use raw types. – Lino Jun 25 '18 at 06:10
  • 1
    There is no apparent value to looping here. Just store the vehicles in a `Map` (or a `List` where vehicles are inserted at their slot index, or even `Vehicle[]`), then you can look up directly if a slot is already assigned. – Andy Turner Jun 25 '18 at 06:29

2 Answers2

0

You're creating an iterator at the initialization of your class. At this point al is probably empty. Leaving you with an empty iterator. As you're never updating the iterator. You will never iterate the list, and thus: The list stays empty.

So replace the use of iter with an enhanced for-loop. Also you seem to want to throw exceptions but you actually never throw them. At last you're comparing Strings with == but this will mostly not work, because == will not check on string equality, but the reference. And this may be not allways true for two strings that seem equal. Use .equals() instead:

public boolean addVehicleToSlot(Vehicle vehicle) {
    if((vehicle.slotNo <=0 || vehicle.slotNo > 40) && (!vehicle.getVehicleType().equals("Car") || !vehicle.getVehicleType().equals("Truck") || !vehicle.getVehicleType().equals("Two Wheeler"))){
        throw new InvalidSlotException("Slot alraedy allotted");
    }

    if(al.isEmpty()){ // instead of al.size() == 0, just use al.isEmpty()
        al.add(vehicle);
        return true;
    }

    for(Vehicle veh : al){
        if(veh.getSlotNo() == vehicle.getSlotNo()){
            throw new SlotNotFoundException("No slot allotted");
        } else{
            al.add(vehicle);
            return true;
        }
    }
    return false;
}
Lino
  • 19,604
  • 6
  • 47
  • 65
  • I would imagine you intend to add the vehicle after the loop. Otherwise, it isn't going to loop very far (it will either throw or return on the first iteration). – Andy Turner Jun 25 '18 at 06:21
  • @AndyTurner in fact I am not sure what the OP wants to achieve with this. The whole looping doesn't really makes sense. – Lino Jun 25 '18 at 06:25
  • If the array is empty then the iterator never will be used so it’s not a problem that it’s empty. – Joakim Danielson Jun 25 '18 at 06:26
  • @JoakimDanielson yes, but after the first insert there will still no iteration happen. Because `itr` is still an empty iterator. Making the impression that the List is *empty* – Lino Jun 25 '18 at 06:26
  • 1
    Ok, that’s a fair point. We don’t really know when and in what scope it gets initialized but maybe you’re right. – Joakim Danielson Jun 25 '18 at 06:30
  • @JoakimDanielson According to the code OP's dumped, it seems that it is declared on class level. But I might be wrong. This question is highly unclear – Lino Jun 25 '18 at 06:31
0

There are quite a few errors and issues you need to take account of:

  • I don't think you understand how iterators work. They are generally very short term variables: declare, iterate, dispose. There are a few use cases for long lasting iterators but this isn't one of them.
  • In fact explicit iterators are not commonly needed these days: you can use a for-each or Stream for most situations.
  • Your logic seems to imply that there can be only one vehicle with a given 'slot' in the list. If that's the case there are much better ways of handling this:

Use a map to store your slots:

Map<Integer,Vehicle> slotMap;

Check with a stream:

if (al.stream().anyMatch(v -> v.getSlotNr() == vehicle.getSlotNr()))
    ...
sprinter
  • 27,148
  • 6
  • 47
  • 78