Code refactor: Remove private class XlsxFormatBorderData

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)
+    , 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)
     , theme(0)
@@ -42,9 +43,9 @@ FormatPrivate::FormatPrivate()
 FormatPrivate::FormatPrivate(const FormatPrivate &other)
     : QSharedData(other)
     , alignmentData(other.alignmentData)
-    , borderData(other.borderData), fillData(other.fillData), protectionData(other.protectionData)
     , dirty(other.dirty), formatKey(other.formatKey)
-    , font_dirty(other.dirty), font_index_valid(other.font_index_valid), font_key(other.font_key), font_index(other.font_index)
+    , font_dirty(other.font_dirty), font_index_valid(other.font_index_valid), font_key(other.font_key), font_index(other.font_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)
     , theme(other.theme)
@@ -594,7 +595,7 @@ void Format::setBorderColor(const QColor &color)
  */
 Format::BorderStyle Format::leftBorderStyle() const
 {
-    return d->borderData.left;
+    return static_cast<BorderStyle>(intProperty(FormatPrivate::P_Border_LeftStyle));
 }
 
 /*!
@@ -602,8 +603,7 @@ Format::BorderStyle Format::leftBorderStyle() const
  */
 void Format::setLeftBorderStyle(BorderStyle style)
 {
-    d->borderData.left = style;
-    d->borderData._dirty = true;
+    setProperty(FormatPrivate::P_Border_LeftStyle, style);
 }
 
 /*!
@@ -611,137 +611,136 @@ void Format::setLeftBorderStyle(BorderStyle style)
  */
 QColor Format::leftBorderColor() const
 {
-    return d->borderData.leftColor;
+    return colorProperty(FormatPrivate::P_Border_LeftColor);
 }
 
 void Format::setLeftBorderColor(const QColor &color)
 {
-    d->borderData.leftColor = color;
-    d->borderData._dirty = true;
+    setProperty(FormatPrivate::P_Border_LeftColor, color);
 }
 
 Format::BorderStyle Format::rightBorderStyle() const
 {
-    return d->borderData.right;
+    return static_cast<BorderStyle>(intProperty(FormatPrivate::P_Border_RightStyle));
 }
 
 void Format::setRightBorderStyle(BorderStyle style)
 {
-    d->borderData.right = style;
-    d->borderData._dirty = true;
+    setProperty(FormatPrivate::P_Border_RightStyle, style);
 }
 
 QColor Format::rightBorderColor() const
 {
-    return d->borderData.rightColor;
+    return colorProperty(FormatPrivate::P_Border_RightColor);
 }
 
 void Format::setRightBorderColor(const QColor &color)
 {
-    d->borderData.rightColor = color;
-    d->borderData._dirty = true;
+    setProperty(FormatPrivate::P_Border_RightColor, color);
 }
 
 Format::BorderStyle Format::topBorderStyle() const
 {
-    return d->borderData.top;
+    return static_cast<BorderStyle>(intProperty(FormatPrivate::P_Border_TopStyle));
 }
 
 void Format::setTopBorderStyle(BorderStyle style)
 {
-    d->borderData.top = style;
-    d->borderData._dirty = true;
+    setProperty(FormatPrivate::P_Border_TopStyle, style);
 }
 
 QColor Format::topBorderColor() const
 {
-    return d->borderData.topColor;
+    return colorProperty(FormatPrivate::P_Border_TopColor);
 }
 
 void Format::setTopBorderColor(const QColor &color)
 {
-    d->borderData.topColor = color;
-    d->borderData._dirty = true;
+    setProperty(FormatPrivate::P_Border_TopColor, color);
 }
 
 Format::BorderStyle Format::bottomBorderStyle() const
 {
-    return d->borderData.bottom;
+    return static_cast<BorderStyle>(intProperty(FormatPrivate::P_Border_BottomStyle));
 }
 
 void Format::setBottomBorderStyle(BorderStyle style)
 {
-    d->borderData.bottom = style;
-    d->borderData._dirty = true;
+    setProperty(FormatPrivate::P_Border_BottomStyle, style);
 }
 
 QColor Format::bottomBorderColor() const
 {
-    return d->borderData.bottomColor;
+    return colorProperty(FormatPrivate::P_Border_BottomColor);
 }
 
 void Format::setBottomBorderColor(const QColor &color)
 {
-    d->borderData.bottomColor = color;
-    d->borderData._dirty = true;
+    setProperty(FormatPrivate::P_Border_BottomColor, color);
 }
 
 Format::BorderStyle Format::diagonalBorderStyle() const
 {
-    return d->borderData.diagonal;
+    return static_cast<BorderStyle>(intProperty(FormatPrivate::P_Border_DiagonalStyle));
 }
 
 void Format::setDiagonalBorderStyle(BorderStyle style)
 {
-    d->borderData.diagonal = style;
-    d->borderData._dirty = true;
+    setProperty(FormatPrivate::P_Border_DiagonalStyle, style);
 }
 
 Format::DiagonalBorderType Format::diagonalBorderType() const
 {
-    return d->borderData.diagonalType;
+    return static_cast<DiagonalBorderType>(intProperty(FormatPrivate::P_Border_DiagonalType));
 }
 
 void Format::setDiagonalBorderType(DiagonalBorderType style)
 {
-    d->borderData.diagonalType = style;
-    d->borderData._dirty = true;
+    setProperty(FormatPrivate::P_Border_DiagonalType, style);
 }
 
 QColor Format::diagonalBorderColor() const
 {
-    return d->borderData.diagonalColor;
+    return colorProperty(FormatPrivate::P_Border_DiagonalColor);
 }
 
 void Format::setDiagonalBorderColor(const QColor &color)
 {
-    d->borderData.diagonalColor = color;
-    d->borderData._dirty = true;
+    setProperty(FormatPrivate::P_Border_DiagonalColor, color);
 }
 
 bool Format::borderIndexValid() const
 {
-    return d->borderData.indexValid();
+    return d->border_index_valid;
 }
 
 int Format::borderIndex() const
 {
-    return d->borderData.index();
+    return d->border_index;
 }
 
 void Format::setBorderIndex(int index)
 {
-    d->borderData.setIndex(index);
+    d->border_index = index;
 }
 
 /* Internal
  */
 QByteArray Format::borderKey() const
 {
-    if (d->borderData._dirty)
-        d->dirty = true; //Make sure formatKey() will be re-generated.
+    if (d->border_dirty) {
+        QByteArray key;
+        QDataStream stream(&key, QIODevice::WriteOnly);
+        for (int i=FormatPrivate::P_Border_STARTID; i<FormatPrivate::P_Border_ENDID; ++i) {
+            if (d->property.contains(i))
+                stream << i << d->property[i];
+        };
+
+        const_cast<Format*>(this)->d->border_key = key;
+        const_cast<Format*>(this)->d->border_dirty = false;
+    }
 
-    return d->borderData.key();
+    return d->border_key;
 }
 
 Format::FillPattern Format::fillPattern() const
@@ -830,7 +829,7 @@ void Format::setLocked(bool locked)
 
 QByteArray Format::formatKey() const
 {
-    if (d->dirty || d->borderData._dirty || d->fillData._dirty) {
+    if (d->dirty || d->fillData._dirty) {
         QByteArray key;
         QDataStream stream(&key, QIODevice::WriteOnly);
         stream<<fontKey()<<borderKey()<<fillKey()
@@ -933,6 +932,11 @@ void Format::setProperty(int propertyId, const QVariant &value)
         d->font_dirty = true;
         d->font_index_valid = false;
     }
+
+    if (propertyId >= FormatPrivate::P_Border_STARTID && propertyId < FormatPrivate::P_Border_ENDID) {
+        d->border_dirty = true;
+        d->border_index_valid = false;
+    }
 }
 
 /*!

src/xlsx/xlsxformat_p.h

@@ -45,72 +45,6 @@ struct XlsxFormatAlignmentData
     bool shinkToFit;
 };
 
-struct XlsxFormatBorderData
-{
-    XlsxFormatBorderData() :
-        left(Format::BorderNone), right(Format::BorderNone), top(Format::BorderNone)
-      ,bottom(Format::BorderNone), diagonal(Format::BorderNone)
-      ,diagonalType(Format::DiagonalBorderNone)
-      ,_dirty(true), _indexValid(false), _index(-1)
-    {}
-
-    Format::BorderStyle left;
-    Format::BorderStyle right;
-    Format::BorderStyle top;
-    Format::BorderStyle bottom;
-    Format::BorderStyle diagonal;
-    QColor leftColor;
-    QColor rightColor;
-    QColor topColor;
-    QColor bottomColor;
-    QColor diagonalColor;
-    QString leftThemeColor;
-    QString rightThemeColor;
-    QString topThemeColor;
-    QString bottomThemeColor;
-    QString diagonalThemeColor;
-    Format::DiagonalBorderType diagonalType;
-
-    QByteArray key() const
-    {
-        if (_dirty) {
-            QByteArray key;
-            QDataStream stream(&key, QIODevice::WriteOnly);
-            stream << bottom << bottomColor << bottomThemeColor << top << topColor << topThemeColor
-                 << diagonal << diagonalColor << diagonalThemeColor << diagonalType
-                << left << leftColor << leftThemeColor << right << rightColor << rightThemeColor;
-            const_cast<XlsxFormatBorderData*>(this)->_key = key;
-            const_cast<XlsxFormatBorderData*>(this)->_dirty = false;
-            const_cast<XlsxFormatBorderData*>(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 XlsxFormatFillData {
     XlsxFormatFillData() :
         pattern(Format::PatternNone)
@@ -212,7 +146,24 @@ public:
         P_Font_ENDID,
 
         //border
-        P_Border_,
+        P_Border_STARTID,
+        P_Border_LeftStyle = P_Border_STARTID,
+        P_Border_RightStyle,
+        P_Border_TopStyle,
+        P_Border_BottomStyle,
+        P_Border_DiagonalStyle,
+        P_Border_LeftColor,
+        P_Border_RightColor,
+        P_Border_TopColor,
+        P_Border_BottomColor,
+        P_Border_DiagonalColor,
+        P_Border_DiagonalType,
+        P_Border_ThemeLeftColor,
+        P_Border_ThemeRightColor,
+        P_Border_ThemeTopColor,
+        P_Border_ThemeBottomColor,
+        P_Border_ThemeDiagonalColor,
+        P_Border_ENDID,
 
         //fill
         P_Fill_,
@@ -229,7 +180,6 @@ public:
     ~FormatPrivate();
 
     XlsxFormatAlignmentData alignmentData;
-    XlsxFormatBorderData borderData;
     XlsxFormatFillData fillData;
     XlsxFormatProtectionData protectionData;
 
@@ -241,6 +191,11 @@ public:
     QByteArray font_key;
     int font_index;
 
+    bool border_dirty;
+    bool border_index_valid;
+    QByteArray border_key;
+    int border_index;
+
     int xf_index;
     bool xf_indexValid;
 

src/xlsx/xlsxstyles.cpp

@@ -173,12 +173,12 @@ void Styles::addFormat(Format *format, bool force)
     //Border
     if (!format->borderIndexValid()) {
         if (!m_bordersHash.contains(format->borderKey())) {
-            QSharedPointer<XlsxFormatBorderData> border = QSharedPointer<XlsxFormatBorderData>(new XlsxFormatBorderData(format->d->borderData));
-            border->setIndex(m_bordersList.size()); //Assign proper index
-            m_bordersList.append(border);
-            m_bordersHash[border->key()] = border;
+            format->setBorderIndex(m_bordersList.size()); //Assign proper index
+            m_bordersList.append(format);
+            m_bordersHash[format->borderKey()] = format;
+        } else {
+            format->setBorderIndex(m_bordersHash[format->borderKey()]->borderIndex());
         }
-        format->setBorderIndex(m_bordersHash[format->borderKey()]->index());
     }
 
     //Format
@@ -444,24 +444,32 @@ void Styles::writeBorders(XmlStreamWriter &writer)
     writer.writeStartElement(QStringLiteral("borders"));
     writer.writeAttribute(QStringLiteral("count"), QString::number(m_bordersList.count()));
     for (int i=0; i<m_bordersList.size(); ++i) {
-        QSharedPointer<XlsxFormatBorderData> border = m_bordersList[i];
+        Format *border = m_bordersList[i];
 
         writer.writeStartElement(QStringLiteral("border"));
-        if (border->diagonalType == Format::DiagonalBorderUp) {
-            writer.writeAttribute(QStringLiteral("diagonalUp"), QStringLiteral("1"));
-        } else if (border->diagonalType == Format::DiagonalBorderDown) {
-            writer.writeAttribute(QStringLiteral("diagonalDown"), QStringLiteral("1"));
-        } else if (border->diagonalType == Format::DiagnoalBorderBoth) {
-            writer.writeAttribute(QStringLiteral("diagonalUp"), QStringLiteral("1"));
-            writer.writeAttribute(QStringLiteral("diagonalDown"), QStringLiteral("1"));
+        if (border->hasProperty(FormatPrivate::P_Border_DiagonalType)) {
+            Format::DiagonalBorderType t = border->diagonalBorderType();
+            if (t == Format::DiagonalBorderUp) {
+                writer.writeAttribute(QStringLiteral("diagonalUp"), QStringLiteral("1"));
+            } else if (t == Format::DiagonalBorderDown) {
+                writer.writeAttribute(QStringLiteral("diagonalDown"), QStringLiteral("1"));
+            } else if (t == Format::DiagnoalBorderBoth) {
+                writer.writeAttribute(QStringLiteral("diagonalUp"), QStringLiteral("1"));
+                writer.writeAttribute(QStringLiteral("diagonalDown"), QStringLiteral("1"));
+            }
         }
-        writeSubBorder(writer, QStringLiteral("left"), border->left, border->leftColor, border->leftThemeColor);
-        writeSubBorder(writer, QStringLiteral("right"), border->right, border->rightColor, border->rightThemeColor);
-        writeSubBorder(writer, QStringLiteral("top"), border->top, border->topColor, border->topThemeColor);
-        writeSubBorder(writer, QStringLiteral("bottom"), border->bottom, border->bottomColor, border->bottomThemeColor);
+        if (border->hasProperty(FormatPrivate::P_Border_LeftStyle))
+            writeSubBorder(writer, QStringLiteral("left"), border->leftBorderStyle(), border->leftBorderColor(), border->stringProperty(FormatPrivate::P_Border_ThemeLeftColor));
+        if (border->hasProperty(FormatPrivate::P_Border_RightStyle))
+            writeSubBorder(writer, QStringLiteral("right"), border->rightBorderStyle(), border->rightBorderColor(), border->stringProperty(FormatPrivate::P_Border_ThemeRightColor));
+        if (border->hasProperty(FormatPrivate::P_Border_TopStyle))
+            writeSubBorder(writer, QStringLiteral("top"), border->topBorderStyle(), border->topBorderColor(), border->stringProperty(FormatPrivate::P_Border_ThemeTopColor));
+        if (border->hasProperty(FormatPrivate::P_Border_BottomStyle))
+            writeSubBorder(writer, QStringLiteral("bottom"), border->bottomBorderStyle(), border->bottomBorderColor(), border->stringProperty(FormatPrivate::P_Border_ThemeBottomColor));
 
 //        if (!format->isDxfFormat()) {
-        writeSubBorder(writer, QStringLiteral("diagonal"), border->diagonal, border->diagonalColor, border->diagonalThemeColor);
+        if (border->hasProperty(FormatPrivate::P_Border_DiagonalStyle))
+            writeSubBorder(writer, QStringLiteral("diagonal"), border->diagonalBorderStyle(), border->diagonalBorderColor(), border->stringProperty(FormatPrivate::P_Border_ThemeDiagonalColor));
 //        }
         writer.writeEndElement();//border
     }
@@ -787,30 +795,60 @@ bool Styles::readBorders(XmlStreamReader &reader)
 bool Styles::readBorder(XmlStreamReader &reader)
 {
     Q_ASSERT(reader.name() == QLatin1String("border"));
-    QSharedPointer<XlsxFormatBorderData> border(new XlsxFormatBorderData);
+    Format *border = createFormat();
 
     QXmlStreamAttributes attributes = reader.attributes();
     bool isUp = attributes.hasAttribute(QLatin1String("diagonalUp"));
     bool isDown = attributes.hasAttribute(QLatin1String("diagonalUp"));
     if (isUp && isDown)
-        border->diagonalType = Format::DiagnoalBorderBoth;
+        border->setDiagonalBorderType(Format::DiagnoalBorderBoth);
     else if (isUp)
-        border->diagonalType = Format::DiagonalBorderUp;
+        border->setDiagonalBorderType(Format::DiagonalBorderUp);
     else if (isDown)
-        border->diagonalType = Format::DiagonalBorderDown;
+        border->setDiagonalBorderType(Format::DiagonalBorderDown);
 
     while((reader.readNextStartElement(), true)) { //read until border endelement
         if (reader.tokenType() == QXmlStreamReader::StartElement) {
-            if (reader.name() == QLatin1String("left"))
-                readSubBorder(reader, reader.name().toString(), border->left, border->leftColor, border->leftThemeColor);
-            else if (reader.name() == QLatin1String("right"))
-                readSubBorder(reader, reader.name().toString(), border->right, border->rightColor, border->rightThemeColor);
-            else if (reader.name() == QLatin1String("top"))
-                readSubBorder(reader, reader.name().toString(), border->top, border->topColor, border->topThemeColor);
-            else if (reader.name() == QLatin1String("bottom"))
-                readSubBorder(reader, reader.name().toString(), border->bottom, border->bottomColor, border->bottomThemeColor);
-            else if (reader.name() == QLatin1String("diagonal"))
-                readSubBorder(reader, reader.name().toString(), border->diagonal, border->diagonalColor, border->diagonalThemeColor);
+            if (reader.name() == QLatin1String("left") || reader.name() == QLatin1String("right")
+                    || reader.name() == QLatin1String("top") || reader.name() == QLatin1String("bottom")
+                    || reader.name() == QLatin1String("diagonal") ) {
+                Format::BorderStyle style(Format::BorderNone);
+                QColor color;
+                QString themeColor;
+                readSubBorder(reader, reader.name().toString(), style, color, themeColor);
+
+                if (reader.name() == QLatin1String("left")) {
+                    border->setLeftBorderStyle(style);
+                    if (color.isValid())
+                        border->setLeftBorderColor(color);
+                    else if (!themeColor.isEmpty())
+                        border->setProperty(FormatPrivate::P_Border_ThemeLeftColor, themeColor);
+                } else if (reader.name() == QLatin1String("right")) {
+                    border->setRightBorderStyle(style);
+                    if (color.isValid())
+                        border->setRightBorderColor(color);
+                    else if (!themeColor.isEmpty())
+                        border->setProperty(FormatPrivate::P_Border_ThemeRightColor, themeColor);
+                } else if (reader.name() == QLatin1String("top")) {
+                    border->setTopBorderStyle(style);
+                    if (color.isValid())
+                        border->setTopBorderColor(color);
+                    else if (!themeColor.isEmpty())
+                        border->setProperty(FormatPrivate::P_Border_ThemeTopColor, themeColor);
+                } else if (reader.name() == QLatin1String("bottom")) {
+                    border->setBottomBorderStyle(style);
+                    if (color.isValid())
+                        border->setBottomBorderColor(color);
+                    else if (!themeColor.isEmpty())
+                        border->setProperty(FormatPrivate::P_Border_ThemeBottomColor, themeColor);
+                } else if (reader.name() == QLatin1String("diagonal")) {
+                    border->setDiagonalBorderStyle(style);
+                    if (color.isValid())
+                        border->setDiagonalBorderColor(color);
+                    else if (!themeColor.isEmpty())
+                        border->setProperty(FormatPrivate::P_Border_ThemeDiagonalColor, themeColor);
+                }
+            }
         }
 
         if (reader.tokenType() == QXmlStreamReader::EndElement && reader.name() == QLatin1String("border"))
@@ -818,8 +856,8 @@ bool Styles::readBorder(XmlStreamReader &reader)
     }
 
     m_bordersList.append(border);
-    m_bordersHash.insert(border->key(), border);
-    border->setIndex(m_bordersList.size()-1);//first call key(), then setIndex()
+    m_bordersHash.insert(border->borderKey(), border);
+    border->setBorderIndex(m_bordersList.size()-1);//first call key(), then setIndex()
 
     return true;
 }
@@ -931,7 +969,11 @@ bool Styles::readCellXfs(XmlStreamReader &reader)
             if (id >= m_bordersList.size()) {
                 qDebug("Error read styles.xml, cellXfs borderId");
             } else {
-                format->d->borderData = *m_bordersList[id];
+                Format *borderFormat = m_bordersList[id];
+                for (int i=FormatPrivate::P_Border_STARTID; i<FormatPrivate::P_Border_ENDID; ++i) {
+                    if (borderFormat->hasProperty(i))
+                        format->setProperty(i, borderFormat->property(i));
+                }
             }
         }
 

src/xlsx/xlsxstyles_p.h

@@ -41,7 +41,6 @@ namespace QXlsx {
 
 class Format;
 struct XlsxFormatFillData;
-struct XlsxFormatBorderData;
 class XmlStreamWriter;
 class XmlStreamReader;
 
@@ -99,10 +98,10 @@ private:
     int m_nextCustomNumFmtId;
     QList<Format *> m_fontsList;
     QList<QSharedPointer<XlsxFormatFillData> > m_fillsList; //Keep a copy of unique fills
-    QList<QSharedPointer<XlsxFormatBorderData> > m_bordersList; //Keep a copy of unique borders
+    QList<Format *> m_bordersList; //Keep a copy of unique borders
     QHash<QByteArray, Format *> m_fontsHash;
     QHash<QByteArray, QSharedPointer<XlsxFormatFillData> > m_fillsHash;
-    QHash<QByteArray, QSharedPointer<XlsxFormatBorderData> > m_bordersHash;
+    QHash<QByteArray, Format *> m_bordersHash;
 
     QVector<QColor> m_indexedColors;
 

tests/auto/styles/tst_stylestest.cpp

@@ -18,6 +18,8 @@ private Q_SLOTS:
     void testAddFormat2();
     void testSolidFillBackgroundColor();
 
+    void testWriteBorders();
+
     void testReadNumFmts();
     void testReadFonts();
     void testReadFills();
@@ -81,6 +83,18 @@ void StylesTest::testSolidFillBackgroundColor()
     QVERIFY(xmlData.contains("<patternFill patternType=\"solid\"><fgColor rgb=\"FFff0000\"/>"));
 }
 
+void StylesTest::testWriteBorders()
+{
+    QXlsx::Styles styles;
+    QXlsx::Format *format = styles.createFormat();
+    format->setRightBorderStyle(QXlsx::Format::BorderThin);
+    styles.addFormat(format);
+
+    QByteArray xmlData = styles.saveToXmlData();
+
+    QVERIFY(xmlData.contains("<border><right style=\"thin\">"));
+}
+
 void StylesTest::testReadFonts()
 {
     QByteArray xmlData = "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"yes\"?>"