Code refactor: Remove private class XlsxFormatFillData

src/xlsx/xlsxformat.cpp

@@ -33,6 +33,7 @@ QT_BEGIN_NAMESPACE_XLSX
 FormatPrivate::FormatPrivate()
     : dirty(true)
     , font_dirty(true), font_index_valid(false), font_index(-1)
+    , fill_dirty(true), fill_index_valid(false), fill_index(-1)
     , border_dirty(true), border_index_valid(false), border_index(-1)
     , xf_index(-1), xf_indexValid(false)
     , is_dxf_fomat(false), dxf_index(-1), dxf_indexValid(false)
@@ -42,9 +43,10 @@ FormatPrivate::FormatPrivate()
 
 FormatPrivate::FormatPrivate(const FormatPrivate &other)
     : QSharedData(other)
-    , alignmentData(other.alignmentData)
+    , alignmentData(other.alignmentData) ,protectionData(other.protectionData)
     , dirty(other.dirty), formatKey(other.formatKey)
     , font_dirty(other.font_dirty), font_index_valid(other.font_index_valid), font_key(other.font_key), font_index(other.font_index)
+    , fill_dirty(other.fill_dirty), fill_index_valid(other.fill_index_valid), fill_key(other.fill_key), fill_index(other.fill_index)
     , border_dirty(other.border_dirty), border_index_valid(other.border_index_valid), border_key(other.border_key), border_index(other.border_index)
     , xf_index(other.xf_index), xf_indexValid(other.xf_indexValid)
     , is_dxf_fomat(other.is_dxf_fomat), dxf_index(other.dxf_index), dxf_indexValid(other.dxf_indexValid)
@@ -745,64 +747,70 @@ QByteArray Format::borderKey() const
 
 Format::FillPattern Format::fillPattern() const
 {
-    return d->fillData.pattern;
+    return static_cast<FillPattern>(intProperty(FormatPrivate::P_Fill_Pattern));
 }
 
 void Format::setFillPattern(FillPattern pattern)
 {
-    d->fillData.pattern = pattern;
+    setProperty(FormatPrivate::P_Fill_Pattern, pattern);
-    d->fillData._dirty = true;
 }
 
 QColor Format::patternForegroundColor() const
 {
-    return d->fillData.fgColor;
+    return colorProperty(FormatPrivate::P_Fill_FgColor);
 }
 
 void Format::setPatternForegroundColor(const QColor &color)
 {
-    if (color.isValid() && d->fillData.pattern == PatternNone)
+    if (color.isValid() && !hasProperty(FormatPrivate::P_Fill_Pattern))
-        d->fillData.pattern = PatternSolid;
+        setFillPattern(PatternSolid);
-    d->fillData.fgColor = color;
+    setProperty(FormatPrivate::P_Fill_FgColor, color);
-    d->fillData._dirty = true;
 }
 
 QColor Format::patternBackgroundColor() const
 {
-    return d->fillData.bgColor;
+    return colorProperty(FormatPrivate::P_Fill_BgColor);
 }
 
 void Format::setPatternBackgroundColor(const QColor &color)
 {
-    if (color.isValid() && d->fillData.pattern == PatternNone)
+    if (color.isValid() && !hasProperty(FormatPrivate::P_Fill_Pattern))
-        d->fillData.pattern = PatternSolid;
+        setFillPattern(PatternSolid);
-    d->fillData.bgColor = color;
+    setProperty(FormatPrivate::P_Fill_BgColor, color);
-    d->fillData._dirty = true;
 }
 
 bool Format::fillIndexValid() const
 {
-    return d->fillData.indexValid();
+    return d->fill_index_valid;
 }
 
 int Format::fillIndex() const
 {
-    return d->fillData.index();
+    return d->fill_index;
 }
 
 void Format::setFillIndex(int index)
 {
-    d->fillData.setIndex(index);
+    d->fill_index = index;
 }
 
 /* Internal
  */
 QByteArray Format::fillKey() const
 {
-    if (d->fillData._dirty)
+    if (d->fill_dirty) {
-        d->dirty = true; //Make sure formatKey() will be re-generated.
+        QByteArray key;
+        QDataStream stream(&key, QIODevice::WriteOnly);
+        for (int i=FormatPrivate::P_Fill_STARTID; i<FormatPrivate::P_Fill_ENDID; ++i) {
+            if (d->property.contains(i))
+                stream << i << d->property[i];
+        };
 
-    return d->fillData.key();
+        const_cast<Format*>(this)->d->fill_key = key;
+        const_cast<Format*>(this)->d->fill_dirty = false;
+    }
+
+    return d->fill_key;
 }
 
 bool Format::hidden() const
@@ -829,7 +837,7 @@ void Format::setLocked(bool locked)
 
 QByteArray Format::formatKey() const
 {
-    if (d->dirty || d->fillData._dirty) {
+    if (d->dirty) {
         QByteArray key;
         QDataStream stream(&key, QIODevice::WriteOnly);
         stream<<fontKey()<<borderKey()<<fillKey()
@@ -931,11 +939,12 @@ void Format::setProperty(int propertyId, const QVariant &value)
     if (propertyId >= FormatPrivate::P_Font_STARTID && propertyId < FormatPrivate::P_Font_ENDID) {
         d->font_dirty = true;
         d->font_index_valid = false;
-    }
+    } else if (propertyId >= FormatPrivate::P_Border_STARTID && propertyId < FormatPrivate::P_Border_ENDID) {
-
-    if (propertyId >= FormatPrivate::P_Border_STARTID && propertyId < FormatPrivate::P_Border_ENDID) {
         d->border_dirty = true;
         d->border_index_valid = false;
+    } else if (propertyId >= FormatPrivate::P_Fill_STARTID && propertyId < FormatPrivate::P_Fill_ENDID) {
+        d->fill_dirty = true;
+        d->fill_index_valid = false;
     }
 }
 

src/xlsx/xlsxformat_p.h

@@ -45,58 +45,6 @@ struct XlsxFormatAlignmentData
     bool shinkToFit;
 };
 
-struct XlsxFormatFillData {
-    XlsxFormatFillData() :
-        pattern(Format::PatternNone)
-      ,_dirty(true), _indexValid(false), _index(-1)
-    {}
-
-    Format::FillPattern pattern;
-    QColor bgColor;
-    QColor fgColor;
-    QString bgThemeColor;
-    QString fgThemeColor;
-
-    QByteArray key() const
-    {
-        if (_dirty) {
-            QByteArray key;
-            QDataStream stream(&key, QIODevice::WriteOnly);
-            stream<< bgColor << bgThemeColor
-                  << fgColor << fgThemeColor
-                  << pattern;
-            const_cast<XlsxFormatFillData*>(this)->_key = key;
-            const_cast<XlsxFormatFillData*>(this)->_dirty = false;
-            const_cast<XlsxFormatFillData*>(this)->_indexValid = false;
-        }
-        return _key;
-    }
-
-    bool indexValid() const
-    {
-        return !_dirty && _indexValid;
-    }
-
-    int index() const
-    {
-        return _index;
-    }
-
-    void setIndex(int index)
-    {
-        _index = index;
-        _indexValid = true;
-    }
-
-    //helper member
-    bool _dirty; //key re-generated and proper index assign is need.
-
-private:
-    QByteArray _key;
-    bool _indexValid;  //has a valid index, so no need to assign a new one
-    int _index; //index in the border list
-};
-
 struct XlsxFormatProtectionData {
     XlsxFormatProtectionData() :
         locked(false), hidden(false)
@@ -166,7 +114,13 @@ public:
         P_Border_ENDID,
 
         //fill
-        P_Fill_,
+        P_Fill_STARTID,
+        P_Fill_Pattern = P_Fill_STARTID,
+        P_Fill_BgColor,
+        P_Fill_FgColor,
+        P_Fill_BgThemeColor,
+        P_Fill_FgThemeColor,
+        P_Fill_ENDID,
 
         //alignment
         P_Alignment_,
@@ -180,7 +134,6 @@ public:
     ~FormatPrivate();
 
     XlsxFormatAlignmentData alignmentData;
-    XlsxFormatFillData fillData;
     XlsxFormatProtectionData protectionData;
 
     bool dirty; //The key re-generation is need.
@@ -191,6 +144,11 @@ public:
     QByteArray font_key;
     int font_index;
 
+    bool fill_dirty;
+    bool fill_index_valid;
+    QByteArray fill_key;
+    int fill_index;
+
     bool border_dirty;
     bool border_index_valid;
     QByteArray border_key;

src/xlsx/xlsxstyles.cpp

@@ -48,10 +48,11 @@ Styles::Styles(bool createEmpty)
         //Add default Format
         addFormat(createFormat());
         //Add another fill format
-        QSharedPointer<XlsxFormatFillData> fill = QSharedPointer<XlsxFormatFillData>(new XlsxFormatFillData);
+        Format *format = createFormat();
-        fill->pattern = Format::PatternGray125;
+        format->setFillPattern(Format::PatternGray125);
-        m_fillsList.append(fill);
+
-        m_fillsHash[fill->key()] = fill;
+        m_fillsList.append(format);
+        m_fillsHash.insert(format->fillKey(), format);
     }
 }
 
@@ -162,12 +163,12 @@ void Styles::addFormat(Format *format, bool force)
     //Fill
     if (!format->fillIndexValid()) {
         if (!m_fillsHash.contains(format->fillKey())) {
-            QSharedPointer<XlsxFormatFillData> fill = QSharedPointer<XlsxFormatFillData>(new XlsxFormatFillData(format->d->fillData));
+            format->setFillIndex(m_fillsList.size()); //Assign proper index
-            fill->setIndex(m_fillsList.size()); //Assign proper index
+            m_fillsList.append(format);
-            m_fillsList.append(fill);
+            m_fillsHash[format->fillKey()] = format;
-            m_fillsHash[fill->key()] = fill;
+        } else {
+            format->setFillIndex(m_fillsHash[format->fillKey()]->fillIndex());
         }
-        format->setFillIndex(m_fillsHash[format->fillKey()]->index());
     }
 
     //Border
@@ -379,13 +380,13 @@ void Styles::writeFills(XmlStreamWriter &writer)
     writer.writeAttribute(QStringLiteral("count"), QString::number(m_fillsList.size()));
 
     for (int i=0; i<m_fillsList.size(); ++i) {
-        QSharedPointer<XlsxFormatFillData> fill = m_fillsList[i];
+        Format *fill = m_fillsList[i];
-        writeFill(writer, fill.data());
+        writeFill(writer, fill);
     }
     writer.writeEndElement(); //fills
 }
 
-void Styles::writeFill(XmlStreamWriter &writer, XlsxFormatFillData *fill)
+void Styles::writeFill(XmlStreamWriter &writer, Format *fill)
 {
     static QMap<int, QString> patternStrings;
     if (patternStrings.isEmpty()) {
@@ -412,24 +413,24 @@ void Styles::writeFill(XmlStreamWriter &writer, XlsxFormatFillData *fill)
 
     writer.writeStartElement(QStringLiteral("fill"));
     writer.writeStartElement(QStringLiteral("patternFill"));
-    writer.writeAttribute(QStringLiteral("patternType"), patternStrings[fill->pattern]);
+    writer.writeAttribute(QStringLiteral("patternType"), patternStrings[fill->fillPattern()]);
     // For a solid fill, Excel reverses the role of foreground and background colours
-    if (fill->fgColor.isValid()) {
+    if (fill->patternForegroundColor().isValid()) {
-        writer.writeEmptyElement(fill->pattern == Format::PatternSolid ? QStringLiteral("bgColor") : QStringLiteral("fgColor"));
+        writer.writeEmptyElement(fill->fillPattern() == Format::PatternSolid ? QStringLiteral("bgColor") : QStringLiteral("fgColor"));
-        writer.writeAttribute(QStringLiteral("rgb"), QStringLiteral("FF")+fill->fgColor.name().mid(1));
+        writer.writeAttribute(QStringLiteral("rgb"), QStringLiteral("FF")+fill->patternForegroundColor().name().mid(1));
-    } else if (!fill->fgThemeColor.isEmpty()) {
+    } else if (!fill->stringProperty(FormatPrivate::P_Fill_FgThemeColor).isEmpty()) {
         writer.writeEmptyElement(QStringLiteral("fgColor"));
-        QStringList themes = fill->fgThemeColor.split(QLatin1Char(':'));
+        QStringList themes = fill->stringProperty(FormatPrivate::P_Fill_FgThemeColor).split(QLatin1Char(':'));
         writer.writeAttribute(QStringLiteral("theme"), themes[0]);
         if (!themes[1].isEmpty())
             writer.writeAttribute(QStringLiteral("tint"), themes[1]);
     }
-    if (fill->bgColor.isValid()) {
+    if (fill->patternBackgroundColor().isValid()) {
-        writer.writeEmptyElement(fill->pattern == Format::PatternSolid ? QStringLiteral("fgColor") : QStringLiteral("bgColor"));
+        writer.writeEmptyElement(fill->fillPattern() == Format::PatternSolid ? QStringLiteral("fgColor") : QStringLiteral("bgColor"));
-        writer.writeAttribute(QStringLiteral("rgb"), QStringLiteral("FF")+fill->bgColor.name().mid(1));
+        writer.writeAttribute(QStringLiteral("rgb"), QStringLiteral("FF")+fill->patternBackgroundColor().name().mid(1));
-    } else if (!fill->bgThemeColor.isEmpty()) {
+    } else if (!fill->stringProperty(FormatPrivate::P_Fill_BgThemeColor).isEmpty()) {
         writer.writeEmptyElement(QStringLiteral("bgColor"));
-        QStringList themes = fill->bgThemeColor.split(QLatin1Char(':'));
+        QStringList themes = fill->stringProperty(FormatPrivate::P_Fill_BgThemeColor).split(QLatin1Char(':'));
         writer.writeAttribute(QStringLiteral("theme"), themes[0]);
         if (!themes[1].isEmpty())
             writer.writeAttribute(QStringLiteral("tint"), themes[1]);
@@ -724,13 +725,15 @@ bool Styles::readFill(XmlStreamReader &reader)
         patternValues[QStringLiteral("lightGrid")] = Format::PatternLightGrid;
     }
 
-    QSharedPointer<XlsxFormatFillData> fill(new XlsxFormatFillData);
+    Format *fill = createFormat();
     while((reader.readNextStartElement(), true)) { //read until fill endelement
         if (reader.tokenType() == QXmlStreamReader::StartElement) {
             if (reader.name() == QLatin1String("patternFill")) {
                 QXmlStreamAttributes attributes = reader.attributes();
+                if (attributes.hasAttribute(QLatin1String("patternType"))) {
                     QString pattern = attributes.value(QLatin1String("patternType")).toString();
-                fill->pattern = patternValues.contains(pattern) ? patternValues[pattern] : Format::PatternNone;
+                    fill->setFillPattern(patternValues.contains(pattern) ? patternValues[pattern] : Format::PatternNone);
+                }
             } else if (reader.name() == QLatin1String("fgColor")) {
                 QXmlStreamAttributes attributes = reader.attributes();
                 QColor c;
@@ -741,12 +744,12 @@ bool Styles::readFill(XmlStreamReader &reader)
                 } else if (attributes.hasAttribute(QLatin1String("theme"))) {
                     QString theme = attributes.value(QLatin1String("theme")).toString();
                     QString tint = attributes.value(QLatin1String("tint")).toString();
-                    fill->fgThemeColor = theme + QLatin1Char(':') + tint;
+                    fill->setProperty(FormatPrivate::P_Fill_FgThemeColor, QString(theme + QLatin1Char(':') + tint));
                 }
-                if (fill->pattern == Format::PatternSolid)
+                if (fill->fillPattern() == Format::PatternSolid)
-                    fill->bgColor = c;
+                    fill->setPatternBackgroundColor(c);
                 else
-                    fill->fgColor = c;
+                    fill->setPatternForegroundColor(c);
             } else if (reader.name() == QLatin1String("bgColor")) {
                 QXmlStreamAttributes attributes = reader.attributes();
                 QColor c;
@@ -757,12 +760,12 @@ bool Styles::readFill(XmlStreamReader &reader)
                 } else if (attributes.hasAttribute(QLatin1String("theme"))) {
                     QString theme = attributes.value(QLatin1String("theme")).toString();
                     QString tint = attributes.value(QLatin1String("tint")).toString();
-                    fill->bgThemeColor = theme + QLatin1Char(':') + tint;
+                    fill->setProperty(FormatPrivate::P_Fill_BgThemeColor, QString(theme + QLatin1Char(':') + tint));
                 }
-                if (fill->pattern == Format::PatternSolid)
+                if (fill->fillPattern() == Format::PatternSolid)
-                    fill->fgColor = c;
+                    fill->setPatternForegroundColor(c);
                 else
-                    fill->bgColor = c;
+                    fill->setPatternBackgroundColor(c);
             }
         }
 
@@ -771,8 +774,8 @@ bool Styles::readFill(XmlStreamReader &reader)
     }
 
     m_fillsList.append(fill);
-    m_fillsHash.insert(fill->key(), fill);
+    m_fillsHash.insert(fill->fillKey(), fill);
-    fill->setIndex(m_fillsList.size()-1);//first call key(), then setIndex()
+    fill->setFillIndex(m_fillsList.size()-1);//first call key(), then setIndex()
 
     return true;
 }
@@ -960,7 +963,11 @@ bool Styles::readCellXfs(XmlStreamReader &reader)
             if (id >= m_fillsList.size()) {
                 qDebug("Error read styles.xml, cellXfs fillId");
             } else {
-                format->d->fillData = *m_fillsList[id];
+                Format *fillFormat = m_fillsList[id];
+                for (int i=FormatPrivate::P_Fill_STARTID; i<FormatPrivate::P_Fill_ENDID; ++i) {
+                    if (fillFormat->hasProperty(i))
+                        format->setProperty(i, fillFormat->property(i));
+                }
             }
         }
 

src/xlsx/xlsxstyles_p.h

@@ -40,7 +40,6 @@ class StylesTest;
 namespace QXlsx {
 
 class Format;
-struct XlsxFormatFillData;
 class XmlStreamWriter;
 class XmlStreamReader;
 
@@ -73,7 +72,7 @@ private:
     void writeNumFmts(XmlStreamWriter &writer);
     void writeFonts(XmlStreamWriter &writer);
     void writeFills(XmlStreamWriter &writer);
-    void writeFill(XmlStreamWriter &writer, XlsxFormatFillData *fill);
+    void writeFill(XmlStreamWriter &writer, Format *fill);
     void writeBorders(XmlStreamWriter &writer);
     void writeSubBorder(XmlStreamWriter &writer, const QString &type, int style, const QColor &color, const QString &themeColor);
     void writeCellXfs(XmlStreamWriter &writer);
@@ -97,10 +96,10 @@ private:
     QHash<QString, QSharedPointer<XlsxFormatNumberData> > m_customNumFmtsHash;
     int m_nextCustomNumFmtId;
     QList<Format *> m_fontsList;
-    QList<QSharedPointer<XlsxFormatFillData> > m_fillsList; //Keep a copy of unique fills
+    QList<Format *> m_fillsList;
-    QList<Format *> m_bordersList; //Keep a copy of unique borders
+    QList<Format *> m_bordersList;
     QHash<QByteArray, Format *> m_fontsHash;
-    QHash<QByteArray, QSharedPointer<XlsxFormatFillData> > m_fillsHash;
+    QHash<QByteArray, Format *> m_fillsHash;
     QHash<QByteArray, Format *> m_bordersHash;
 
     QVector<QColor> m_indexedColors;

tests/auto/styles/tst_stylestest.cpp

@@ -34,7 +34,6 @@ 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");
 }
@@ -128,8 +127,8 @@ void StylesTest::testReadFills()
     styles.readFills(reader);
 
     QCOMPARE(styles.m_fillsList.size(), 4);
-    QCOMPARE(styles.m_fillsList[3]->pattern, QXlsx::Format::PatternSolid);
+    QCOMPARE(styles.m_fillsList[3]->fillPattern(), QXlsx::Format::PatternSolid);
-    QCOMPARE(styles.m_fillsList[3]->bgColor, QColor(Qt::gray));//for solid pattern, bg vs. fg color!
+    QCOMPARE(styles.m_fillsList[3]->patternBackgroundColor(), QColor(Qt::gray));//for solid pattern, bg vs. fg color!
 }
 
 void StylesTest::testReadBorders()