Finding indices of CSV headers in Java

Question

I'm working on a project where I needed to extract the indices for the headers of a CSV file to initialise objects. I thought that I should work on my documentation process and learn how to document code, seeing as I'm always reading about people not documenting code.

/**
 * Returns the indices of searched for headers, e.g. if the file looks like this: (Name Email
 * Phone Age) and we call the method: findIndices("Name", "Age", "Email", "Phone"), if returns
 * [0,3,1,2].
 *
 * Ignores case of letters, i.e. Age and AGE "is" the same.
 *
 * @param input Strings to search for in the header
 * @return int[] with indices for the columns
*/
public int[] findIndices(String... input) throws IllegalArgumentException {
    int[] indices = new int[input.length];
    if (input.length > this.headers.length) {
      throw new IllegalArgumentException("Amount of searched for headers, " + input.length
      + ", exceeded the actual amount of headers: " + this.headers.length);
    }
    boolean found = true; // Assume we will find all headers.
    int k = 0; // Counter variable.
    for (int i = 0; i < this.headers.length; i++) {
        for (String s : input) {
            if (s.equalsIgnoreCase(this.headers[i])) {
                indices[k++] = i;
                found = true;
            } else {
                found = false;
            }
        }
    }
    if (!found) {
        throw new IllegalArgumentException(
            "One or more of input arguments could not be found in the headers.");
    }
    return indices;
}

My own thoughts:

1. The found flag seems poorly handled.
2. This is part of a custom CSV-reader class, with a headers private field. This seems a bit weird to me, this could maybe be static and take a String[] as input as well.
3. Should this actually throw an exception or should it just return a "bad" index? But I'm using an array to represent it. Null, maybe?

Edit: Found a bug which did not show up in my unit tests due to a misunderstanding of the JUnit api, found -> !found.

Answer 1

Since you mentioned unit tests, here are 2 unit tests that proves your code does not work (Based on name, age, email, phone header):

@Test
public void testFindIndicesThrowsException()
{
    ClassName x = new ClassName();

    try
    {
        int[] result = x.findIndices("Name", "age", "FieldThatDoesNotExist", "Phone");
        fail("expected exception was not thrown!");
    }
    catch (IllegalArgumentException exception)
    {
        assertEquals("One or more of input arguments could not be found in the headers.", exception.getMessage());
    }
}

@Test
public void testFindIndices()
{
    ClassName x = new ClassName();

    int[] result = x.findIndices("Email", "Phone", "Name", "age");
    assertEquals(4, result.length);
    assertEquals(2, result[0]);
    assertEquals(3, result[1]);
    assertEquals(0, result[2]);
    assertEquals(1, result[3]);
}

Explanation: You are setting a boolean in a loop. This means it could be set to false on one iteration, then set to true on the next.

Instead, you can set the index to the one found, and if it's not found, throw an error:

int k = 0;

for (String s : input) 
{
    indices[k] = -1;

    for (int i = 0; i < this.headers.length; i++)
    {
        if (s.equalsIgnoreCase(this.headers[i])) 
        {
            indices[k] = i;
        }
    }

    // If we never found the item
    if (indices[k] == -1)
    {
        throw new IllegalArgumentException(
            "One or more of input arguments could not be found in the headers.");
    }

    k++;
}

Answer 2

Too me the biggest thing to look at in your code is the complexity. Right now it's O(n²). This is very inefficient and can be greatly improved upon, by taking a step back and changing the headers to a HashMap<String, Integer>. Now, since lookups for HashMap is O(1) you only need to count the loop through the input array, which will give a complexity of O(n). The code could look something like this:

HashMap<String, Integer> headers = new HashMap<>();

public int[] findIndices(String... input) throws IllegalArgumentException {
    int headerSize = headers.size();
    if (input.length > headerSize) {
        throw new IllegalArgumentException("Amount of searched for headers, " + input.length
                + ", exceeded the actual amount of headers: " + headerSize);
    }
    int[] indices = new int[input.length];
    int index = 0;
    for (String s : input) {
        if (headers.containsKey(s)) {
            indices[index++] = headers.get(s);
        } else {
            throw new IllegalArgumentException("One or more of input arguments could not be found in the headers.");
        }
    }
    return indices;
}

Answer 3

    if (input.length > this.headers.length) {
      throw new IllegalArgumentException("Amount of searched for headers, " + input.length
      + ", exceeded the actual amount of headers: " + this.headers.length);
    }

Why is this needed? At the moment I get [0,1,0] for findIndices("name", "age", "name"), but IllegalArgumentException when I call findIndices("name", "age", "name", "email", "phone").