From 46704458883210be1b37ade8c3be01a19ba32321 Mon Sep 17 00:00:00 2001 From: Debao Zhang Date: Thu, 21 Nov 2013 14:36:18 +0800 Subject: [PATCH] Code refactor: Remove private class XlsxFormatBorderData --- src/xlsx/xlsxformat.cpp | 88 +++++++++++---------- src/xlsx/xlsxformat_p.h | 91 ++++++---------------- src/xlsx/xlsxstyles.cpp | 112 ++++++++++++++++++--------- src/xlsx/xlsxstyles_p.h | 5 +- tests/auto/styles/tst_stylestest.cpp | 14 ++++ 5 files changed, 162 insertions(+), 148 deletions(-) diff --git a/src/xlsx/xlsxformat.cpp b/src/xlsx/xlsxformat.cpp index 77cb8a1..d3a023f 100755 --- a/src/xlsx/xlsxformat.cpp +++ b/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(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(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(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(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(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(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; iproperty.contains(i)) + stream << i << d->property[i]; + }; + + const_cast(this)->d->border_key = key; + const_cast(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<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; + } } /*! diff --git a/src/xlsx/xlsxformat_p.h b/src/xlsx/xlsxformat_p.h index e5c2d8d..6048bdf 100644 --- a/src/xlsx/xlsxformat_p.h +++ b/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(this)->_key = key; - const_cast(this)->_dirty = false; - const_cast(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; diff --git a/src/xlsx/xlsxstyles.cpp b/src/xlsx/xlsxstyles.cpp index 9c78888..19cead9 100755 --- a/src/xlsx/xlsxstyles.cpp +++ b/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 border = QSharedPointer(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 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 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; ihasProperty(i)) + format->setProperty(i, borderFormat->property(i)); + } } } diff --git a/src/xlsx/xlsxstyles_p.h b/src/xlsx/xlsxstyles_p.h index 0b43f9c..2492134 100755 --- a/src/xlsx/xlsxstyles_p.h +++ b/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 m_fontsList; QList > m_fillsList; //Keep a copy of unique fills - QList > m_bordersList; //Keep a copy of unique borders + QList m_bordersList; //Keep a copy of unique borders QHash m_fontsHash; QHash > m_fillsHash; - QHash > m_bordersHash; + QHash m_bordersHash; QVector m_indexedColors; diff --git a/tests/auto/styles/tst_stylestest.cpp b/tests/auto/styles/tst_stylestest.cpp index 03c237b..6118984 100644 --- a/tests/auto/styles/tst_stylestest.cpp +++ b/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("")); } +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("")); +} + void StylesTest::testReadFonts() { QByteArray xmlData = ""