Finding the array indices of three specific headers

Question

I'm considering how to refactor code to make it clean & clear.

There are some principles to make code clean; in this case I think it's Single Responsibility and One level of abstraction.

The original code is like this:

for (int i = 0; i < header.length; i++) {
 if (header[i].equals(Constant.ID)) {
 indexOfID = i;
 } else if (header[i].equals(Constant.NAME)) {
 indexOfName = i;
 } else if (header[i].equals(Constant.CATEGORY)) {
 indexOfCategory = i;
 }
}

Then I separated the loop into smaller functions.

private int getIndexOfColumnType(String[] header, String type) {
 int index = -1;
 int i = 0;
 int size = header.length;
 while (index < size && index < 0) {
 if(header[i].equals(type)) {
 index = i;
 }
 i++;
 }
 return index;
}

And call it

int indexOfID = getIndexOfColumnType(header, Constant.ID);
int indexOfName = getIndexOfColumnType(header, Constant.NAME);
int indexOfCategory = getIndexOfColumnType(header, Constant.CATEGORY);

In this way, I need to use 3 loops; so I wonder which is better for performance because header array can be large.

Please tell me your idea.

• mdfst13: I'd also want to see a lot more code. For example, where is header populated?
• Timothy Truckle: when ever you need an if in your code you most likey should better use polymorphism to solve the situation. But the code you posted is way too short to give a concrete proposal.
• Manh Le: This is small piece of code in a function I extract from my project. The header is header of CSV file, so the size can be 2, 3 to ~100.

Answer 1

Both one-for-loop and getIndexOfColumnTypesolutions represent a naive approach: its main drawback is that for each index you need a dedicated variable like indexOfID or indexOfName.

Since you confirm that the header array can be quite large, this also means that you'll need as many variables for indices as there are items in the array.

Of course, there might be more flexible solutions and I see that the first step is already in the original code, supposing that Constant is an enumeration like this:

public enum Constant {
 ID,
 NAME,
 CATEGORY;
}

The index of each column can be mapped with a dedicated EnumMap:

Map<Constant, Integer> indices = new EnumMap<>(Constant.class);

Now this map can be filled with the indices, using a single loop and they will be accessible by the references to the respective Constants:

private void calculateColumnIndices() {
 for (int i = 0; i < header.length; i++) {
 try {
 Constant column = Constant.valueOf(header[i]);
 indices.put(column, i);
 } catch (IllegalArgumentException ex) {
 // handle the exception depending on what you expect to do if the header contains an invalid column name
 // TODO
 }
 }
}

To access the index:

int index = indices.get(Constant.CATEGORY);

P.S. I'd also suggest to rename Constant to something more meaningful, e.g. ColumnName.

• Manh Le: Thank you for your suggestion. But in my case Constant is Class, which contain common value (not only column name), not an Enum. So, how to improve the code?
• Antot: You can always create this enum without changing/breaking the existing class. Or even use a Map<String, Integer>, but enum is obviously better for this case.