Refactor reported design and code smell issues

fastexcel-core/pom.xml

@@ -0,0 +1,17 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+
+    <parent>
+        <groupId>org.dhatim</groupId>
+        <artifactId>fastexcel-parent</artifactId>
+        <version>0-SNAPSHOT</version>
+        <relativePath>../pom.xml</relativePath>
+    </parent>
+
+    <artifactId>fastexcel-core</artifactId>
+    <name>Fastexcel Core</name>
+    <packaging>jar</packaging>
+</project>

fastexcel-core/src/main/java/module-info.java

@@ -0,0 +1,3 @@
+module org.dhatim.fastexcel.core {
+    exports org.dhatim.fastexcel.common;
+}

fastexcel-core/src/main/java/org/dhatim/fastexcel/common/CellAddressUtil.java

@@ -0,0 +1,31 @@
+package org.dhatim.fastexcel.common;
+
+import java.nio.charset.StandardCharsets;
+
+public final class CellAddressUtil {
+
+    private static final int COL_RADIX = 'Z' - 'A' + 1;
+    private static final int MAX_COL_CHARS = 3;
+
+    private CellAddressUtil() {
+    }
+
+    public static String convertNumToColString(int col) {
+        int excelColNum = col + 1;
+
+        final byte[] colRef = new byte[MAX_COL_CHARS];
+        int colRemain = excelColNum;
+        int pos = MAX_COL_CHARS - 1;
+        while (colRemain > 0) {
+            int thisPart = colRemain % COL_RADIX;
+            if (thisPart == 0) {
+                thisPart = COL_RADIX;
+            }
+            colRemain = (colRemain - thisPart) / COL_RADIX;
+
+            colRef[pos--] = (byte) (thisPart + (int) 'A' - 1);
+        }
+        pos++;
+        return new String(colRef, pos, (MAX_COL_CHARS - pos), StandardCharsets.ISO_8859_1);
+    }
+}

fastexcel-reader/pom.xml

@@ -11,6 +11,10 @@
     <url>https://github.com/dhatim/fastexcel</url>
 
     <dependencies>
+        <dependency>
+            <groupId>org.dhatim</groupId>
+            <artifactId>fastexcel-core</artifactId>
+        </dependency>
         <dependency>
             <groupId>com.fasterxml</groupId>
             <artifactId>aalto-xml</artifactId>

fastexcel-reader/src/main/java/module-info.java

@@ -1,7 +1,8 @@
 module org.dhatim.fastexcel.reader {
+    requires org.dhatim.fastexcel.core;
     requires java.xml;
     requires java.logging;
     requires org.apache.commons.compress;
     requires com.fasterxml.aalto;
     exports org.dhatim.fastexcel.reader;
-}
+}

fastexcel-reader/src/main/java/org/dhatim/fastexcel/reader/BaseFormulaCell.java

@@ -1,6 +1,6 @@
 package org.dhatim.fastexcel.reader;
 
-public class BaseFormulaCell {
+class BaseFormulaCell {
     private final CellAddress baseCelAddr;
 
     private final String formula;

fastexcel-reader/src/main/java/org/dhatim/fastexcel/reader/Cell.java

@@ -24,8 +24,8 @@ public class Cell {
 
     private static final long DAY_MILLISECONDS = 86_400_000L;
 
-    private final ReadableWorkbook workbook;
-    private final Object value;
+    private final boolean date1904;
+    private final CellValue value;
     private final String formula;
     private final CellType type;
     private final CellAddress address;
@@ -39,9 +39,9 @@ public class Cell {
 
     Cell(ReadableWorkbook workbook, CellType type, Object value, CellAddress address, String formula, String rawValue,
          String dataFormatId, String dataFormatString) {
-        this.workbook = workbook;
+        this.date1904 = workbook != null && workbook.isDate1904();
         this.type = type;
-        this.value = value;
+        this.value = CellValue.of(type, value);
         this.address = address;
         this.formula = formula;
         this.rawValue = rawValue;
@@ -62,7 +62,7 @@ public CellAddress getAddress() {
     }
 
     public Object getValue() {
-        return value;
+        return value.asObject();
     }
 
     /**
@@ -78,7 +78,7 @@ public String getFormula() {
 
     public BigDecimal asNumber() {
         requireType(CellType.NUMBER);
-        return (BigDecimal) value;
+        return value.asNumber();
     }
 
     /**
@@ -107,7 +107,7 @@ private LocalDateTime convertToDate(double value) {
 
         int startYear = 1900;
         int dayAdjust = -1; // Excel thinks 2/29/1900 is a valid date, which it isn't
-        if (workbook.isDate1904()) {
+        if (date1904) {
             startYear = 1904;
             dayAdjust = 1; // 1904 date windowing uses 1/2/1904 as the first day
         } else if (wholeDays < 61) {
@@ -122,7 +122,7 @@ private LocalDateTime convertToDate(double value) {
 
     public Boolean asBoolean() {
         requireType(CellType.BOOLEAN);
-        return (Boolean) value;
+        return value.asBoolean();
     }
 
     /**
@@ -132,7 +132,7 @@ public Boolean asBoolean() {
      */
     public String asString() {
         requireType(CellType.STRING);
-        return value == null ? "" : (String) value;
+        return value.asString();
     }
 
     private void requireType(CellType requiredType) {
@@ -146,7 +146,7 @@ private void requireType(CellType requiredType) {
      * @see #asString()
      */
     public String getText() {
-        return value == null ? "" : value.toString();
+        return value.asText();
     }
 
     public Integer getDataFormatId() {
@@ -167,12 +167,95 @@ public String getDataFormatString() {
     public String toString() {
         StringBuilder sb = new StringBuilder();
         sb.append('[').append(type).append(' ');
-        if (value == null) {
+        if (value.asObject() == null) {
             sb.append("null");
         } else {
-            sb.append('"').append(value).append('"');
+            sb.append('"').append(value.asObject()).append('"');
         }
         return sb.append(']').toString();
     }
 
+    private interface CellValue {
+        Object asObject();
+
+        default BigDecimal asNumber() {
+            return (BigDecimal) asObject();
+        }
+
+        default Boolean asBoolean() {
+            return (Boolean) asObject();
+        }
+
+        default String asString() {
+            Object value = asObject();
+            return value == null ? "" : (String) value;
+        }
+
+        default String asText() {
+            Object value = asObject();
+            return value == null ? "" : value.toString();
+        }
+
+        static CellValue of(CellType type, Object value) {
+            switch (type) {
+                case NUMBER:
+                    return new NumberCellValue((BigDecimal) value);
+                case BOOLEAN:
+                    return new BooleanCellValue((Boolean) value);
+                case STRING:
+                    return new StringCellValue((String) value);
+                default:
+                    return new ObjectCellValue(value);
+            }
+        }
+    }
+
+    private static final class ObjectCellValue implements CellValue {
+        private final Object value;
+
+        private ObjectCellValue(Object value) {
+            this.value = value;
+        }
+
+        public Object asObject() {
+            return value;
+        }
+    }
+
+    private static final class NumberCellValue implements CellValue {
+        private final BigDecimal value;
+
+        private NumberCellValue(BigDecimal value) {
+            this.value = value;
+        }
+
+        public Object asObject() {
+            return value;
+        }
+    }
+
+    private static final class BooleanCellValue implements CellValue {
+        private final Boolean value;
+
+        private BooleanCellValue(Boolean value) {
+            this.value = value;
+        }
+
+        public Object asObject() {
+            return value;
+        }
+    }
+
+    private static final class StringCellValue implements CellValue {
+        private final String value;
+
+        private StringCellValue(String value) {
+            this.value = value;
+        }
+
+        public Object asObject() {
+            return value;
+        }
+    }
+
 }

fastexcel-reader/src/main/java/org/dhatim/fastexcel/reader/CellAddress.java

@@ -15,7 +15,8 @@
  */
 package org.dhatim.fastexcel.reader;
 
-import java.nio.charset.StandardCharsets;
+import org.dhatim.fastexcel.common.CellAddressUtil;
+
 import java.util.Objects;
 
 public final class CellAddress implements Comparable<CellAddress> {
@@ -114,25 +115,7 @@ static StringBuilder format(StringBuilder sb, int row, int col) {
     }
 
     public static String convertNumToColString(int col) {
-        // Excel counts column A as the 1st column, we
-        // treat it as the 0th one
-        int excelColNum = col + 1;
-
-        final int MAX_COL_CHARS = 3;
-        final byte[] colRef = new byte[MAX_COL_CHARS];
-        int colRemain = excelColNum;
-        int pos = 2;
-        while (colRemain > 0) {
-            int thisPart = colRemain % COL_RADIX;
-            if (thisPart == 0) {
-                thisPart = COL_RADIX;
-            }
-            colRemain = (colRemain - thisPart) / COL_RADIX;
-
-            colRef[pos--] = (byte) (thisPart + (int) 'A' - 1);
-        }
-        pos++;
-        return new String(colRef, pos, (MAX_COL_CHARS - pos), StandardCharsets.ISO_8859_1);
+        return CellAddressUtil.convertNumToColString(col);
     }
 
 

fastexcel-reader/src/main/java/org/dhatim/fastexcel/reader/DefaultXMLInputFactory.java

@@ -2,8 +2,14 @@
 
 import javax.xml.stream.XMLInputFactory;
 
-public class DefaultXMLInputFactory {
-    static final XMLInputFactory factory = defaultXmlInputFactory();
+final class DefaultXMLInputFactory {
+
+    private DefaultXMLInputFactory() {
+    }
+
+    static XMLInputFactory create() {
+        return defaultXmlInputFactory();
+    }
 
     private static XMLInputFactory defaultXmlInputFactory() {
         XMLInputFactory factory  = new com.fasterxml.aalto.stax.InputFactoryImpl();

fastexcel-reader/src/main/java/org/dhatim/fastexcel/reader/FormatCatalog.java

@@ -0,0 +1,34 @@
+package org.dhatim.fastexcel.reader;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+
+final class FormatCatalog {
+
+    static final FormatCatalog EMPTY = new FormatCatalog(Collections.emptyList(), Collections.emptyMap());
+
+    private final List<String> formatIdsByStyleIndex;
+    private final Map<String, String> formatsById;
+
+    FormatCatalog(List<String> formatIdsByStyleIndex, Map<String, String> formatsById) {
+        this.formatIdsByStyleIndex = Collections.unmodifiableList(formatIdsByStyleIndex);
+        this.formatsById = Collections.unmodifiableMap(formatsById);
+    }
+
+    String getFormatId(int styleIndex) {
+        return styleIndex < formatIdsByStyleIndex.size() ? formatIdsByStyleIndex.get(styleIndex) : null;
+    }
+
+    String getFormatString(String formatId) {
+        return formatsById.get(formatId);
+    }
+
+    List<String> getFormatIdsByStyleIndex() {
+        return formatIdsByStyleIndex;
+    }
+
+    Map<String, String> getFormatsById() {
+        return formatsById;
+    }
+}