Even Though using of Generics I had distinguish the T type

Question

I like to examine a Excel .csv column which may be of type String,Int oder Double. I implement a classic generic Pair class:

public class PairT<K,V> implements Comparable<PairT<K,V>>

In case of an integer column the column values are stored in:

ArrayList<PairT<Integer,Integer>> column_list = new ArrayList<>();

where the V value holds the excel row index.

Code snippet below shows the nasty solution - I would improve:

// Add a cell value of type T to the column list
@SuppressWarnings("unchecked")
public static <T> void addCell(
        String excelcell,
        int row_idx,
        boolean ignorecase,
        T defkey,
        /*IO*/ArrayList<PairT<T,Integer>> column_list) throws RuntimeException
{   
    //Class<?> classtype = defkey.getClass();  String typename = classtype.getSimpleName();
    char type;
    if (defkey      instanceof String) type = 'S';
    else if (defkey instanceof Integer) type = 'I';
    else if (defkey instanceof Double) type = 'D';
    else type = 'O'; // other

    T key;
try
{
    switch(type)
    {
    case 'I': 
        try 
        {  key = (T)new Integer(excelcell);
        } catch (NumberFormatException e) { key = defkey; }  

        column_list.add(new PairT<T,Integer>(key,row_idx));
        break;         
    case 'D':
        try 
        {   key = (T)new Double(excelcell);          
        } catch (NumberFormatException e) { key = defkey; } 

        column_list.add(new PairT<T,Integer>(key,row_idx));
        break;
    case 'S':
        if (ignorecase) excelcell = excelcell.toUpperCase();
        column_list.add(new PairT<T,Integer>((T)excelcell,row_idx));
        break;
    default:  // Other take the .toString() output as key 
        column_list.add(new PairT<T,Integer>((T)excelcell.toString(),row_idx));
    }
}catch (Exception ex) // possibly a ClassCastException
{
    throw new RuntimeException("addCell(): Problems using PairT<K,V>",ex);
}
} //----- end of addCell()

To test I use:

ArrayList<PairT<Integer,Integer>> column_list = new ArrayList<>();

int row_idx = 0; 

boolean ic = true; // for String values only;
Integer defval = new Integer("0");

String cell = "12";
addCell(cell,row_idx,ic,defval,column_list);

cell = "17.34"; // leads to def val
addCell(cell,++row_idx,ic,defval,column_list);

cell = "456";
addCell(cell,++row_idx,ic,defval,column_list);

cell = "foo"; // lead to def avlue
addCell(cell,++row_idx,ic,defval,column_list);

System.out.println("result: " + column_list); 
// [12;0, 0;1, 456;2, 0;3]

java.util.Collections.sort(column_list);  
System.out.println("Sorted: " + column_list); 

//Sorted: [0;1, 0;3, 12;0, 456;2]

It works as expected, however -as I said- I dont want to distinguish the Type T in addCell(). I would prefer a short solution, something like:

if (ignorecase) column_list.add(new PairT<T,Integer>((T)excelcell.toUpperCase(),row_idx));
else            column_list.add(new PairT<T,Integer>((T)excelcell,row_idx));

FYI, K and V as types in a map stand for Key and Value. They're not really appropriate identifiers in a pair class because there is no key or value. — Michael, Jun 12 '17 at 13:56
You should be able to get away with the diamond operator: `list.add(new PairT<>(foo,foo))` — Michael, Jun 12 '17 at 14:01
The default switch case causes heap pollution by casting `String` to `T`. You should throw e.g. an `IllegalArgumentException` instead. — Radiodef, Jun 12 '17 at 14:04
The cleanest solution I can think of is to create three separate `addCell` methods, with `Integer defKey`, `Double defKey`, and `String defKey` parameters, respectively. This allows you to do away with `T` entirely, which should simplify the code a lot. — VGR, Jun 12 '17 at 14:27

score 1 · Answer 1 · answered Jun 12 '17 at 14:41

You've got a few problems in your code as it stands;

Firstly, you define your column_list as being a list of pairs of Integer values. You want to define it instead as ArrayList<PairT<T, Integer>> column_list = new ArrayList<>(); to allow you to store String, Double or Integer data.

Secondly, in addCell() you check the type of an Integer defval which will always have the value of 0. You then use the type inferred from this variable in the following switch statement, meaning you execute the code for an Integer no matter what the type of excelCell.

Taking these things into account, I have cleaned up the code an written addCell() using generic type arguments.

public class Main {

    @SuppressWarnings("unchecked")
    public static <T> void main(String[] args) {
        ArrayList<PairT<T, Integer>> list = new ArrayList<>();

        int row_idx = 0;

        String cell = "12";
        addCell((T)cell, row_idx, list);

        cell = "17.34";
        addCell((T)cell, ++row_idx, list);

        cell = "456";
        addCell((T)cell, ++row_idx, list);

        cell = "foo";
        addCell((T)cell, ++row_idx, list);

        System.out.println("result: " + list);
        java.util.Collections.sort(list);
        System.out.println("Sorted: " + list);

    }

    @SuppressWarnings("unchecked")
    public static <T> void addCell(T excelCell, int row_idx, ArrayList<PairT<T, Integer>> list){
        if(excelCell instanceof String) list.add((PairT<T, Integer>) new PairT<>(((String) excelCell).toUpperCase(), row_idx));
        else list.add(new PairT<>(excelCell, row_idx));
    }

I have removed ignoreCase and defval as they are irrelevant for this example.

Hallo Luke, guessing that yout casting (T)cell is a simple cell.toString() — Norbert, Jun 13 '17 at 16:15
Yes, i currently checks this. Casting (T)anything simply use the toString(). May be the generic concept suffers in an implements InitFromString method. E.g. a Generic must implement such a method. For exampe — Norbert, Jun 13 '17 at 16:32
Sorry, missing example in last posting: A Generic class Pig had to implement initFromString() to initialize a Pig. E.g. initFromTring("name_of_the_pig; weight;age_of_the_pig",';'); This is the opposite of toString() commonly used. — Norbert, Jun 13 '17 at 16:51
@Norbert Sorry I'm not entirely sure what you're asking? You shouldn't have to change the type of cell to String as the point of generics is that they can take any class defined at compile-time. — Lucy Stevens, Jun 14 '17 at 09:19

score 0 · Accepted Answer · answered Jun 12 '17 at 15:35

You could use something like I described in my answer here and map Class<T> to Function<String, T>, but I'm tending to agree with VGR's comment that you should reconsider the way you're using generics here.

One simple way to refactor your code, which I think VGR is suggesting, would be like this:

public static void addCell
   (String  excelCell,
    int     rowIdx,
    boolean ignoreCase,
    Integer defCellValue,
    List<PairT<Integer, Integer>> columnList)
{
    Integer cellValue;
    try {
        cellValue = Integer.valueOf(excelCell);
    } catch (NumberFormatException x) {
        cellValue = defCellValue;
    }
    columnList.add(new PairT<>(cellValue, rowIdx));
}

// addCell overload with Double defCellValue
// addCell overload with String defCellValue

You could generalize a bit and do something like this, although it doesn't gain you that much if you only have a couple of overloads:

public static void addCell
   (String  excelCell,
    int     rowIdx,
    boolean ignoreCase,
    Integer defCellValue,
    List<PairT<Integer, Integer>> columnList)
{
    addCell(excellCell,
            rowIdx,
            ignoreCase,
            defCellValue,
            columnList,
            Integer::valueOf);
}

public static void addCell(..., Double defCellValue, ...) {
    addCell(..., Double::valueOf);
}

public static void addCell(..., String defCellValue, ...) {
    addCell(..., Function.identity());
}

private static <T> void addCell
   (String  excelCell,
    int     rowIdx,
    boolean ignoreCase,
    T       defCellValue,
    List<PairT<T, Integer>> columnList,
    Function<String, T>     cellParser)
{
    if (ignoreCase) {
        excelCell = excelCell.toUpperCase();
    }
    T cellValue;
    try {
        cellValue = cellParser.apply(excelCell);
    } catch (IllegalArgumentException x) {
        cellValue = defValue;
    }
    columnList.add(new PairT<>(cellValue, rowIdx));
}

A couple other things:

It would make more sense to write e.g. a class Cell<T> { int row, col; T value; } instead of trying to use a pair class as you're doing. The pair class is very noisy, difficult to work with and you can't write cell-specific utility methods in the pair class.
You should stick to Java naming conventions. Static constants are in uppercase with underscores for spaces (static final int FOO_COUNT;) and all other variables are in camel case, with the first letter being lowercase (int fooCount;).

Yes, thanks for the great idea of using such a "Function cellParser" technique (like function pointers in C). I will try this. The second yes: I use a Cell class in the past, but overruling this because of the praised ideas of Generics and the possibly "overkill" for a simple cell — Norbert, Jun 13 '17 at 17:09

Even Though using of Generics I had distinguish the T type

2 Answers2