ShridharBavannawar
ShridharBavannawar

Reputation: 174

How to refactor this switch case

I'm trying to read the data from excel and extracting the data from each column and storing it in another variable. Each case represents the column. Using fbDataField I'm adding the row data into the List name fbList. Can this code be refactored to make it look a clean code?

for(int i=0;i< workbook.getNumberOfSheets(); i++){

    Sheet sheet = workbook.getSheetAt(i);

    Iterator<Row> rowIterator = sheet.iterator();

    while(rowIterator.hasNext()){

        FacebookFields fbDataField= new FacebookFields();
        Row row = rowIterator.next();
        if(row.getRowNum()== 0 )
        {
            continue;
        }
        Iterator<Cell> cellIterator = row.cellIterator();

        while(cellIterator.hasNext()){

            Cell cell= cellIterator.next();

            switch(cell.getColumnIndex()){
            case 0: fbDataField.setName(cell.getStringCellValue());
            break;
            case 1: fbDataField.setId(cell.getStringCellValue());
            break;
            case 2: fbDataField.setDate(cell.getStringCellValue());
            break;
            case 3: fbDataField.setMessage(cell.getStringCellValue());
            break;
            case 4: fbDataField.setType(cell.getStringCellValue());
            break;
            case 5: fbDataField.setPage(cell.getStringCellValue());
            break;
            case 6: fbDataField.setLikeCount(String.valueOf(cell.getNumericCellValue()));
            break;
            case 7: fbDataField.setCommentCount(String.valueOf(cell.getNumericCellValue()));
            break;
            case 8: fbDataField.setShareCount(String.valueOf(cell.getNumericCellValue()));
            break;

            }

        } 
        fbList.add(fbDataField);
    }
}

Upvotes: 2

Views: 853

Answers (2)

OldCurmudgeon
OldCurmudgeon

Reputation: 65811

I'd use an enum as a copy factory.

enum CellToFb {

    Name {

                @Override
                void copy(FacebookFields fb, Cell cell) {
                    fb.setName(cell.getStringCellValue());
                }
            },
    Id {

                @Override
                void copy(FacebookFields fb, Cell cell) {
                    fb.setId(cell.getStringCellValue());
                }
            };

    // Every enum must be able to copy one cell to the correct FB field.
    public abstract void copy(FacebookFields fb, Cell cell);
}

public void test() {
    // Get all copiers - one for each column.
    CellToFb[] copiers = CellToFb.values();
    while (cellIterator.hasNext()) {
        Cell cell = cellIterator.next();
        // Call the correct copier.
        copiers[cell.getColumnIndex()].copy(fbDataField, cell);
    }
}

Each enum knows how to copy a specific column. Just make sure the enums match up with the columns. Your copy code becomes very simple.

This technique ports nicely to Java 8 lambdas.

// Couple of simple demo classes.
class From {

    String id;
    String name;

    @Override
    public String toString() {
        return "{" + id + "," + name + "}";
    }
}

class To {

    String identifier;
    String value;

    @Override
    public String toString() {
        return "{" + identifier + "," + value + "}";
    }
}

enum FromToTo implements BiConsumer<To, From> {

    // Each field defines it's own copying lambda.
    Id((to, from) -> to.identifier = from.id),
    Value((to, from) -> to.value = from.name);
    // The copying lambda.
    private final BiConsumer<To, From> copy;

    private FromToTo(BiConsumer<To, From> copy) {
        this.copy = copy;
    }

    @Override
    public void accept(To to, From from) {
        // Delegate to the copying lambda.
        copy.accept(to, from);
    }

    // Take s static copy to avoid duplicating.
    private static final FromToTo[] copiers = FromToTo.values();

    public static void copy(To to, From from) {
        // Useful function to copy fields from -> to
        Arrays.stream(copiers)
                .forEach(ftt -> ftt.accept(to, from));
    }

}

public void test() {
    From f = new From();
    f.id = "id";
    f.name = "name";
    To t = new To();
    FromToTo.copy(t, f);
    System.out.println(f + " -> " + t);
}

Upvotes: 3

sehe
sehe

Reputation: 393064

The documentation for POI¹ states that it has iterator() aliases to enable foreach loops²:

for (Sheet sheet : workbook) {
    for (Row row : sheet) {

        if (row.getRowNum() == 0) {
            continue;
        }

        FacebookFields record = new FacebookFields();

        for (Cell cell : row) {
            switch (cell.getColumnIndex()) {
                case 0: record.setName        (cell.getStringCellValue()); break; 
                case 1: record.setId          (cell.getStringCellValue()); break; 
                case 2: record.setDate        (cell.getStringCellValue()); break; 
                case 3: record.setMessage     (cell.getStringCellValue()); break; 
                case 4: record.setType        (cell.getStringCellValue()); break; 
                case 5: record.setPage        (cell.getStringCellValue()); break; 
                case 6: record.setLikeCount   (String.valueOf(cell.getNumericCellValue())); break; 
                case 7: record.setCommentCount(String.valueOf(cell.getNumericCellValue())); break; 
                case 8: record.setShareCount  (String.valueOf(cell.getNumericCellValue())); break; 
            }
        }

        fbList.add(record);
    }
}

Note:

  1. I'd consider changing the types of the numerical fields to not be String
  2. I'd consider extracting methods to read a row and to read a worksheet:
for (Sheet sheet : workbook) {
    readSheet(sheet, fbList);
}

private static void readSheet(final Sheet sheet, final List<FacebookFields> fbList) {
    for (Row row : sheet)
        if (row.getRowNum() > 0)
            fbList.add(readRow(row));
}

private static FacebookFields readRow(final Row row) {
    FacebookFields record = new FacebookFields();

    for (Cell cell : row) {
        switch (cell.getColumnIndex()) {
            case 0: record.setName        (cell.getStringCellValue()); break; 
            case 1: record.setId          (cell.getStringCellValue()); break; 
            case 2: record.setDate        (cell.getStringCellValue()); break; 
            case 3: record.setMessage     (cell.getStringCellValue()); break; 
            case 4: record.setType        (cell.getStringCellValue()); break; 
            case 5: record.setPage        (cell.getStringCellValue()); break; 
            case 6: record.setLikeCount   (cell.getNumericCellValue()); break; 
            case 7: record.setCommentCount(cell.getNumericCellValue()); break; 
            case 8: record.setShareCount  (cell.getNumericCellValue()); break; 
        }
    }

    return record;
}

¹ e.g. https://poi.apache.org/apidocs/org/apache/poi/hssf/usermodel/HSSFWorkbook.html

² How does the Java 'for each' loop work?

Upvotes: 3

Related Questions