Code refactor: numFmt part of QXlsx::Format

src/xlsx/xlsxformat.cpp

@@ -45,7 +45,7 @@ FormatPrivate::FormatPrivate()
 
 FormatPrivate::FormatPrivate(const FormatPrivate &other)
     : QSharedData(other)
-    , numberData(other.numberData), fontData(other.fontData), alignmentData(other.alignmentData)
+    , fontData(other.fontData), alignmentData(other.alignmentData)
     , borderData(other.borderData), fillData(other.fillData), protectionData(other.protectionData)
     , dirty(other.dirty), formatKey(other.formatKey)
     , xf_index(other.xf_index), xf_indexValid(other.xf_indexValid)
@@ -107,7 +107,7 @@ Format::~Format()
  */
 int Format::numberFormatIndex() const
 {
-    return d->numberData.formatIndex;
+    return intProperty(FormatPrivate::P_NumFmt_Id);
 }
 
 /*!
@@ -117,9 +117,8 @@ int Format::numberFormatIndex() const
  */
 void Format::setNumberFormatIndex(int format)
 {
-    d->dirty = true;
-    d->numberData.formatIndex = format;
-    d->numberData._valid = true;
+    setProperty(FormatPrivate::P_NumFmt_Id, format);
+    clearProperty(FormatPrivate::P_NumFmt_FormatCode);
 }
 
 /*!
@@ -129,7 +128,7 @@ void Format::setNumberFormatIndex(int format)
  */
 QString Format::numberFormat() const
 {
-    return d->numberData.formatString;
+    return stringProperty(FormatPrivate::P_NumFmt_FormatCode);
 }
 
 /*!
@@ -140,9 +139,8 @@ void Format::setNumberFormat(const QString &format)
 {
     if (format.isEmpty())
         return;
-    d->dirty = true;
-    d->numberData.formatString = format;
-    d->numberData._valid = false; //formatIndex must be re-generated
+    setProperty(FormatPrivate::P_NumFmt_FormatCode, format);
+    clearProperty(FormatPrivate::P_NumFmt_Id); //numFmt id must be re-generated.
 }
 
 /*!
@@ -150,37 +148,32 @@ void Format::setNumberFormat(const QString &format)
  */
 bool Format::isDateTimeFormat() const
 {
-    if (d->numberData._valid && d->numberData.formatString.isEmpty()) {
-        int idx = d->numberData.formatIndex;
-        //Built in date time number index
-        if ((idx >= 15 && idx <= 22) || (idx >= 45 && idx <= 47))
-            return true;
-    } else {
+    if (hasProperty(FormatPrivate::P_NumFmt_FormatCode)) {
+        //Custom numFmt, so
         //Gauss from the number string
-        QString formatCode = d->numberData.formatString;
+        QString formatCode = numberFormat();
         formatCode.remove(QRegularExpression(QStringLiteral("\\[(Green|White|Blue|Magenta|Yellow|Cyan|Red)\\]")));
         if (formatCode.contains(QRegularExpression(QStringLiteral("[dmhys]"))))
             return true;
+    } else if (hasProperty(FormatPrivate::P_NumFmt_Id)){
+        //Non-custom numFmt
+        int idx = numberFormatIndex();
+
+        //Is built-in date time number id?
+        if ((idx >= 15 && idx <= 22) || (idx >= 45 && idx <= 47))
+            return true;
     }
-    return false;
-}
 
-/*!
- * \internal
- */
-bool Format::numFmtIndexValid() const
-{
-    return d->numberData._valid;
+    return false;
 }
 
 /*!
- * \internal
+ * Set a custom num \a format with the given \a id.
  */
-void Format::setNumFmt(int index, const QString &string)
+void Format::setNumberFormat(int id, const QString &format)
 {
-    d->numberData.formatIndex = index;
-    d->numberData.formatString = string;
-    d->numberData._valid = true;
+    setProperty(FormatPrivate::P_NumFmt_Id, id);
+    setProperty(FormatPrivate::P_NumFmt_FormatCode, format);
 }
 
 /*!
@@ -860,7 +853,7 @@ QByteArray Format::formatKey() const
         QByteArray key;
         QDataStream stream(&key, QIODevice::WriteOnly);
         stream<<fontKey()<<borderKey()<<fillKey()
-             <<d->numberData.formatIndex
+             <<numberFormatIndex()
             <<d->alignmentData.alignH<<d->alignmentData.alignV<<d->alignmentData.indent
            <<d->alignmentData.rotation<<d->alignmentData.shinkToFit<<d->alignmentData.wrap
           <<d->protectionData.hidden<<d->protectionData.locked;
@@ -925,4 +918,113 @@ int Format::theme() const
     return d->theme;
 }
 
+/*!
+ * \internal
+ */
+QVariant Format::property(int propertyId) const
+{
+    if (d->property.contains(propertyId))
+        return d->property[propertyId];
+    return QVariant();
+}
+
+/*!
+ * \internal
+ */
+void Format::setProperty(int propertyId, const QVariant &value)
+{
+    if (value.isValid())
+        d->property[propertyId] = value;
+    else
+        d->property.remove(propertyId);
+    d->dirty = true;
+}
+
+/*!
+ * \internal
+ */
+void Format::clearProperty(int propertyId)
+{
+    d->property.remove(propertyId);
+    d->dirty = true;
+}
+
+/*!
+ * \internal
+ */
+bool Format::hasProperty(int propertyId) const
+{
+    return d->property.contains(propertyId);
+}
+
+/*!
+ * \internal
+ */
+bool Format::boolProperty(int propertyId) const
+{
+    if (!hasProperty(propertyId))
+        return false;
+
+    const QVariant prop = d->property[propertyId];
+    if (prop.userType() != QMetaType::Bool)
+        return false;
+    return prop.toBool();
+}
+
+/*!
+ * \internal
+ */
+int Format::intProperty(int propertyId) const
+{
+    if (!hasProperty(propertyId))
+        return 0;
+
+    const QVariant prop = d->property[propertyId];
+    if (prop.userType() != QMetaType::Int)
+        return 0;
+    return prop.toInt();
+}
+
+/*!
+ * \internal
+ */
+double Format::doubleProperty(int propertyId) const
+{
+    if (!hasProperty(propertyId))
+        return 0;
+
+    const QVariant prop = d->property[propertyId];
+    if (prop.userType() != QMetaType::Double && prop.userType() != QMetaType::Float)
+        return 0;
+    return prop.toDouble();
+}
+
+/*!
+ * \internal
+ */
+QString Format::stringProperty(int propertyId) const
+{
+    if (!hasProperty(propertyId))
+        return QString();
+
+    const QVariant prop = d->property[propertyId];
+    if (prop.userType() != QMetaType::QString)
+        return QString();
+    return prop.toString();
+}
+
+/*!
+ * \internal
+ */
+QColor Format::colorProperty(int propertyId) const
+{
+    if (!hasProperty(propertyId))
+        return QColor();
+
+    const QVariant prop = d->property[propertyId];
+    if (prop.userType() != qMetaTypeId<QColor>())
+        return QColor();
+    return qvariant_cast<QColor>(prop);
+}
+
 QT_END_NAMESPACE_XLSX

src/xlsx/xlsxformat.h

@@ -141,6 +141,7 @@ public:
     void setNumberFormatIndex(int format);
     QString numberFormat() const;
     void setNumberFormat(const QString &format);
+    void setNumberFormat(int id, const QString &format);
     bool isDateTimeFormat() const;
 
     int fontSize() const;
@@ -215,6 +216,17 @@ public:
     bool operator == (const Format &format) const;
     bool operator != (const Format &format) const;
 
+    QVariant property(int propertyId) const;
+    void setProperty(int propertyId, const QVariant &value);
+    void clearProperty(int propertyId);
+    bool hasProperty(int propertyId) const;
+
+    bool boolProperty(int propertyId) const;
+    int intProperty(int propertyId) const;
+    double doubleProperty(int propertyId) const;
+    QString stringProperty(int propertyId) const;
+    QColor colorProperty(int propertyId) const;
+
 private:
     friend class Styles;
     friend class Worksheet;
@@ -223,9 +235,6 @@ private:
     friend class SharedStrings;
     friend class ::FormatTest;
 
-    bool numFmtIndexValid() const;
-    void setNumFmt(int index, const QString &string);
-
     bool fontIndexValid() const;
     int fontIndex() const;
     void setFontIndex(int index);

src/xlsx/xlsxformat_p.h

@@ -26,19 +26,10 @@
 #define XLSXFORMAT_P_H
 #include "xlsxformat.h"
 #include <QSharedData>
+#include <QHash>
 
 namespace QXlsx {
 
-struct XlsxFormatNumberData
-{
-    XlsxFormatNumberData() : formatIndex(0), _valid(true) {}
-
-    int formatIndex;
-    QString formatString;
-
-    bool _valid;
-};
-
 struct XlsxFormatFontData
 {
     XlsxFormatFontData() :
@@ -254,11 +245,42 @@ struct XlsxFormatProtectionData {
 class FormatPrivate : public QSharedData
 {
 public:
+    enum FormatType
+    {
+        FT_Invalid = 0,
+        FT_NumFmt = 0x01,
+        FT_Font = 0x02,
+        FT_Alignment = 0x04,
+        FT_Border = 0x08,
+        FT_Fill = 0x10,
+        FT_Protection = 0x20
+    };
+
+    enum Property {
+        //numFmt
+        P_NumFmt_Id,
+        P_NumFmt_FormatCode,
+
+        //font
+        P_Font_,
+
+        //alignment
+        P_Alignment_,
+
+        //border
+        P_Border_,
+
+        //fill
+        P_Fill_,
+
+        //protection
+        P_Protection_,
+    };
+
     FormatPrivate();
     FormatPrivate(const FormatPrivate &other);
     ~FormatPrivate();
 
-    XlsxFormatNumberData numberData;
     XlsxFormatFontData fontData;
     XlsxFormatAlignmentData alignmentData;
     XlsxFormatBorderData borderData;
@@ -276,6 +298,8 @@ public:
     mutable bool dxf_indexValid;
 
     int theme;
+
+    QHash<int, QVariant> property;
 };
 
 }

src/xlsx/xlsxstyles.cpp

@@ -88,7 +88,7 @@ void Styles::addFormat(Format *format, bool force)
         return;
 
     //numFmt
-    if (!format->numFmtIndexValid()) {
+    if (format->hasProperty(FormatPrivate::P_NumFmt_FormatCode) && !format->hasProperty(FormatPrivate::P_NumFmt_Id)) {
         if (m_builtinNumFmtsHash.isEmpty()) {
             m_builtinNumFmtsHash.insert(QStringLiteral("General"), 0);
             m_builtinNumFmtsHash.insert(QStringLiteral("0"), 1);
@@ -131,14 +131,16 @@ void Styles::addFormat(Format *format, bool force)
         const QString str = format->numberFormat();
         //Assign proper number format index
         if (m_builtinNumFmtsHash.contains(str)) {
-            format->setNumFmt(m_builtinNumFmtsHash[str], str);
+            format->setNumberFormat(m_builtinNumFmtsHash[str], str);
         } else if (m_customNumFmtsHash.contains(str)) {
-            format->setNumFmt(m_customNumFmtsHash[str]->formatIndex, str);
+            format->setNumberFormat(m_customNumFmtsHash[str]->formatIndex, str);
         } else {
             //Assign a new fmt Id.
-            format->setNumFmt(m_nextCustomNumFmtId, str);
+            format->setNumberFormat(m_nextCustomNumFmtId, str);
 
-            QSharedPointer<XlsxFormatNumberData> fmt(new XlsxFormatNumberData(format->d->numberData));
+            QSharedPointer<XlsxFormatNumberData> fmt(new XlsxFormatNumberData);
+            fmt->formatIndex = m_nextCustomNumFmtId;
+            fmt->formatString = str;
             m_customNumFmtIdMap.insert(m_nextCustomNumFmtId, fmt);
             m_customNumFmtsHash.insert(str, fmt);
 
@@ -897,7 +899,7 @@ bool Styles::readCellXfs(XmlStreamReader &reader)
             if (!m_customNumFmtIdMap.contains(numFmtIndex))
                 format->setNumberFormatIndex(numFmtIndex);
             else
-                format->d->numberData = *m_customNumFmtIdMap[numFmtIndex];
+                format->setNumberFormat(numFmtIndex, m_customNumFmtIdMap[numFmtIndex]->formatString);
         }
 
         if (xfAttrs.hasAttribute(QLatin1String("applyFont"))) {

src/xlsx/xlsxstyles_p.h

@@ -40,13 +40,20 @@ class StylesTest;
 namespace QXlsx {
 
 class Format;
-struct XlsxFormatNumberData;
 struct XlsxFormatFontData;
 struct XlsxFormatFillData;
 struct XlsxFormatBorderData;
 class XmlStreamWriter;
 class XmlStreamReader;
 
+struct XlsxFormatNumberData
+{
+    XlsxFormatNumberData() : formatIndex(0) {}
+
+    int formatIndex;
+    QString formatString;
+};
+
 class XLSX_AUTOTEST_EXPORT Styles
 {
 public:

tests/auto/styles/tst_stylestest.cpp

@@ -32,6 +32,7 @@ void StylesTest::testEmptyStyle()
 {
     QXlsx::Styles styles;
     QByteArray xmlData = styles.saveToXmlData();
+    qDebug()<<xmlData;
 
     QVERIFY2(xmlData.contains("<cellXfs count=\"1\"><xf numFmtId=\"0\" fontId=\"0\" fillId=\"0\" borderId=\"0\" xfId=\"0\"/></cellXfs>"), "Must have one cell style");
 }