Best way to avoid magic numbers in my case

Question

I'm reading a file that has many columns (22) and I'm using openCSV for reading the file.

each line is represented as a string array nextLine[]

I will have to process/validate the columns and don't want to refer to them as number (i.e. nextLine[0] ... nextLine[22])

I would prefer to refer them as nextLine[COLUMN_A] nextLine[COLUMN_B] ..etc.

My initial approach is to create a singleton using an enum

public enum Columns {
    INSTANCE;
    public int COLUMN_A = 0;  
    ....
    public int COLUMN_X = 22; 
}

then I can refer to the array as:

nextLine[Columns.INSTANCE.COLUMN_A]

Question

Would this be a best approach? I only doubt because I have another model class which simply has getters/setters for all the columns, and now creating another class (singleton) to represent columns as index seems like an added effort.

For the example above, If I had a model class like

public class Columns {
  private String columnA;
  public Columns (String columnA) {
    this.columnA = columnA;
  } 
  public void setColumnA(String columnA) {
    this.columnA = columnA;
  }
  public String getColumnA() {
    return this.columnA;
  }
} 

Could I somehow use nextLine[columnA] rather than create a singleton enum class?

Answer 1

An enum isn't a singleton btw. A singleton is a specific software design pattern which enum's aren't traditionally thought of as such. Your use of the Enum is strange to say the least. If you want to use an enum to refer to specific columns you can simply do the following:

public enum Column {
    public int index;

    A(0), B(1), C(2), D(3);

    public Column( int index ) {
        this.index = index;
    }
}

Then you just say:

String columnAValue = csv[row][Column.A.index]

Doing it that way also allows you to iterate over all of the columns like this:

for( Column column : Column.values ) {
    String column = csv[row][column.index];
}

You would not be able to do that using the pattern you used which makes using the Enum not worth it. If you were going to continue to do what you were doing just make them regular constants at the top of the file:

public class CsvParser {
    public static final int COLUMN_A = 0;
    public static final int COLUMN_B = 1;
    public static final int COLUMN_C = 2;

}

There's no difference between that and the enum approach you used except that it's more straight forward and doesn't involve defining yet another enum.

Now to your question. Is this the best pattern to follow? Well as all architecture type questions it depends. Are you building a program that has to do specific validation on each column of the CSV? Maybe Column A is a integer and has to used as an integer, and column B is a string, and column C is an Enum, etc. Or, you have to attach specific logic to each column then yes this pattern makes sense if the format of your data is always predictable. If you have to support multiple formats of data, but they are fixed (ie only format1, format2, format3) then you can continue to follow this pattern.

However, if you have to read any type of csv format, but attach some fixed amount of parsing and or logic to it then you'd have to read in some metadata about your csv to know which columns are numbers, which are strings, etc. And your logic could be attached by looking at the metadata. Of course this is more flexible, but defining metadata and reading that in and handling it is non-trivial. If you don't need this model don't do it. The other model is much less work and just as robust.

If you look at this from the big picture. In the first architecture we HAVE metadata. It's the Enum or the constants we created in the program. So the metadata is built into the program. In the 2nd style we moved the metadata out of the program into an external representation so it's not baked into the program. The user can change the metadata at runtime where in the first version the metadata can't be changed.

Answer 2

After thinking some more I think this is what you could do:

public class CsvParser {

    private File file;
    private boolean hasHeader = false;
    private Map<String,Integer> columnIndexes;
    private List<String[]> rows = new ArrayList<String[]>();

    public CsvParser( File file, boolean hasHeader ) throws IOException {
        this.file = file;
        this.hasHeader = hasHeader;
    }

    // use this to parse the header from the file
    private void parseColumns(LineNumberReader reader) {
        String line = reader.nextLine();
        if( line != null ) {
            String[] columns = line.split(",");
            setColumns( columns );
        }
    }

    // use this if there is no header in the data.
    public void setColumns( String[] columns ) {
        columnIndex = new HashMap<String,Integer>();
        for( int i = 0; i < columns.length; i++ ) columnIndexes.put( columns.trim(), i);
    }

    public void parse() throws IOException {
        LineNumberReader reader = new LineNumberReader( new FileReader( file ) );
        try {
           if( hasHeader ) parseColumns(reader);
           while( (line = reader.nextLine()) != null ) {
              rows.add( line.split(",") );
           }
        } finally {
           reader.close();
        }
    }

    public Collection<String> getColumns() {
       return columnIndexes.keys();
    }

    public int size() {
       return rows.size();
    }

    public int getInt( int row, String column ) {
       return Integer.parseInt(getString(row,column));
    }

    public String getString( int row, String column ) {
       return rows.get(row)[columnIndexes.get(column)];
    }

    public double getDouble( int row, String column ) {
       return Double.parseDouble(getString(row,column));
    }

    public float getFloat( int row, String column ) {
       return Float.parseFloat(getString(row,column));
    }
}

Answer 3

With 22 columns, I expect you do not process the columns individually but rather in a schematic way. Hence, I would avoid indices altogether and do

for (String columnElement : nextLine) {
  // process columnElement
}

If each column has an individual, specific meaning, an array (or List or Map) is not the best design, either. Then I would rather go with an enum or class modelling each line (or use a suitable framework instead of reinventing the wheel). I wouldn't write (g,s)etters for each line, but rather use polymorphism such that the enum/class knows how to process each column. Or even better: Have a class for a line that delegates the processing to objects of type Column, e.g.

class Line {
  List<Column> columns;

  public void processLine() {
    for (Column c: columns) {
      c.processColumn();
  }

}

class Column {
  public void processColumn() {
    ...
  }
}

Answer 4

I would suggest using a HashMap

In your case could be HashMap<String, String[]> map;

Then you could get your values like:

String[] valuesIAmInterested =  map.get(columnA);

Better yet you could really use your object Column as the key for your map.

HashMap<Column, String[]>   map;