Array Index out of bounds

Question

This is what I'm trying to do: I'm reading a file in from the command line. The file contains a list of data,below this paragraph is what it looks like. The problem I'm having is with the if statements.

import java.util.*;
import java.io.*;

public class VehicleTest {
    public static void main(String[] args) throws FileNotFoundException {
        String vehicle = "vehicle";
        String car = "car";
        String americanCar = "american car";
        String foreignCar = "foreign car";
        String truck = "truck";
        String bicycle = "bicycle";

        File file = new File(args[0]);
        Scanner input = new Scanner(file);

        String[] autos = new String[100];
        ArrayList allVehicles = new ArrayList();


        for (int i = 0; i < autos.length; i++) {
            autos[i] = input.nextLine();
        }

        int j = 0;
        int i = 0;

        while (i++ < autos.length) {
            if (vehicle.equalsIgnoreCase(autos[j++])) {
                Vehicle v = new Vehicle();
                v.setOwnerName(autos[j]);
                allVehicles.add(v);
            }else if(car.equalsIgnoreCase(autos[j++])){
                Car c = new Car();
                c.setOwnerName(autos[j]);
                allVehicles.add(c);
            }
        }

        for(Object a: allVehicles){
            System.out.println(a);
        }
    }
}

In pseudo code this would be:

while i is less than the length of the string  array  
 if you see the word vehicle create a new vehicle object and add it to the arrayList.  
 if you see the word car create a new car object and add it to the arrayList.  
.....

The problems is that I get an arrayOutOfBounds exception with the code I'm using.

I understand that j++ is what is wrong, but how else am I supposed to iterate through the string array so that I can read each line and create the appropriate objects? I'm at a loss as to what to do. I need some help.

foreign car  
aMarioy  
Mario's house  
(777) 777-7777  
gmario@mario.com  
false  
black  
Italy  
4415.91  

truck  
aDougy  
Doug's house  
(123) 456-7890  
hdoug@doug.com  
30  
61234.56  
8/10/2003  

vehicle  
aRobby  
Rob's house  
(987) 654-3210  
irob@rob.com  

bicycle  
bTommy  
Tom's house  
(246) 810-1214  
jtom@tom.com  
7  

truck  
bGeorge  
George's house  
(666) 666-6666  
kgeorge@george.com  
25  
51234.56  
12/4/2004  

vehicle  
bTim  
Tim's house  
(111) 111-1111  
tim@tim.com  

bicycle  
bJim  
Jim's house  
(555) 555-5555  
Ajim@jim.com  
5  

american car  
bJohn  
John's house  
(888) 888-8888  
Bjohn@john.com  
true  
green  
false  
true  

car  
cKen  
Ken's house  
(999) 999-9999  
Cken@ken.com  
false  
orange  

foreign car  
cMario  
Mario's house  
(777) 777-7777  
Dmario@mario.com  
false  
black  
Italy  
4415.91  


american car  
gSam  
Sam's house  
(333) 333-3333  
Hsam@sam.com  
false  
blue  
true  
false  

Answer 1

A couple of problems:

• You're incrementing j in both "if" tests. I haven't checked to be certain (it's quite convoluted code, to be honest) but if you make sure that you only increment j when you've found a match, that will help.
• Your test using i basically means it will try to read as many vehicles as there are lines in the file, rather than stopping when you've reached the end of the file. Basically you don't need i here.

Here's one changed version:

    while (j < autos.length) {
        if (vehicle.equalsIgnoreCase(autos[j])) {
            j++;
            Vehicle v = new Vehicle();
            v.setOwnerName(autos[j++]);
            allVehicles.add(v);
        } else if(car.equalsIgnoreCase(autos[j])){
            j++;
            Car c = new Car();
            c.setOwnerName(autos[j++]);
            allVehicles.add(c);
        }
    }

It would be slightly cleaner to extract the type once though - then you can do the comparisons separately:

    while (j < autos.length) {
        String type = autos[j++];
        if (vehicle.equalsIgnoreCase(type)) {
            Vehicle v = new Vehicle();
            v.setOwnerName(autos[j++]);
            allVehicles.add(v);
        } else if(car.equalsIgnoreCase(type)){
            Car c = new Car();
            c.setOwnerName(autos[j++]);
            allVehicles.add(c);
        }
    }

It's still not quite how I'd do it, but it's closer...

My next step would be to use the scanner more appropriately:

while (scanner.hasNext()) {
    String type = scanner.nextLine();
    if (type.equalsIgnoreCase("vehicle")) {
        allVehicles.add(new Vehicle(scanner));
    } else if (type.equalsIgnoreCase("car")) {
        allVehicles.add(new Car(scanner));
    }
    // ...
}

Then make the constructor for Vehicle, Car etc do the parsing directly from the scanner.

The next step would be to separate out the construction from the iteration. Introduce a new method:

// Use a base type in real code
private static Object parseNextVehicle(Scanner scanner) {
    String type = scanner.nextLine();
    if (type.equalsIgnoreCase("vehicle")) {
        return new Vehicle(scanner);
    } else if (type.equalsIgnoreCase("car")) {
        return new Car(scanner);
    }
    // ... throw an exception indicating an unknown vehicle type
}

// ... and then in the main method, use it like this:
while (scanner.hasNextLine()) {
    allVehicles.add(parseNextVehicle(scanner));
}

Answer 2

Don't use j++ in the subscript; increment it once after the entire loop, rather than once or twice depending on which condition holds.

Probably better to do this:

• replace all your in-line increments (x++) with increment statements (x = x + 1)
• figure out where they need to go to make the code do what you want
• turn them back to inline pre/post increments once you've gotten it working, if it seems appropriate

Answer 3

Every line that does not equal “vehicle” will (incorrectly) increment j so approximately after line 50 you will get the exception.

There are multiple solutions to this:

• Increment j only once per loop.
• Read the lines into another ArrayList.
• Don’t read the lines into a data structure but process them as they are being read. This way you are more independent of the size of your data.

Answer 4

Put your increments and decrements in their own statement. This will make the code easier to understand in most cases.

In your case, j++ is called twice if the first if fails. This is probably not what you want.

I would convert your while loop into a for loop, like so:

for (int i = 0, j = 0; i < autos.length; ++i, ++j) {
    if (vehicle.equalsIgnoreCase(autos[j])) {
        // ...

If i == j always, just use the same variable for both.