Code refactoring: Cache worksheetNames

src/xlsx/xlsxdocument.cpp

@@ -462,11 +462,7 @@ void Document::setCurrentWorksheet(const QString &name)
 bool Document::selectWorksheet(const QString &name)
 {
     Q_D(Document);
-    for (int i=0; i<d->workbook->worksheetCount(); ++i) {
+    return d->workbook->setActiveWorksheet(worksheetNames().indexOf(name));
-        if (d->workbook->worksheet(i)->sheetName() == name)
-            return d->workbook->setActiveWorksheet(i);
-    }
-    return false;
 }
 
 /*!
@@ -475,11 +471,7 @@ bool Document::selectWorksheet(const QString &name)
 QStringList Document::worksheetNames() const
 {
     Q_D(const Document);
-    QStringList names;
+    return d->workbook->worksheetNames();
-    for (int i=0; i<d->workbook->worksheetCount(); ++i)
-        names.append(d->workbook->worksheet(i)->sheetName());
-
-    return names;
 }
 
 /*!

src/xlsx/xlsxworkbook.cpp

@@ -163,6 +163,15 @@ Worksheet *Workbook::addWorksheet(const QString &name)
     return insertWorkSheet(d->worksheets.size(), name);
 }
 
+/*!
+ * \internal
+ */
+QStringList Workbook::worksheetNames() const
+{
+    Q_D(const Workbook);
+    return d->worksheetNames;
+}
+
 /*!
  * \internal
  * Used only when load the xlsx file!!
@@ -175,6 +184,7 @@ Worksheet *Workbook::addWorksheet(const QString &name, int sheetId)
 
     Worksheet *sheet = new Worksheet(name, sheetId, this);
     d->worksheets.append(QSharedPointer<Worksheet>(sheet));
+    d->worksheetNames.append(name);
     return sheet;
 }
 
@@ -184,27 +194,19 @@ Worksheet *Workbook::insertWorkSheet(int index, const QString &name)
     QString worksheetName = name;
     if (!name.isEmpty()) {
         //If user given an already in-used name, we should not continue any more!
-        for (int i=0; i<d->worksheets.size(); ++i) {
+        if (d->worksheetNames.contains(name))
-            if (d->worksheets[i]->sheetName() == name) {
             return 0;
-            }
-        }
     } else {
-        bool exists;
         do {
             ++d->last_sheet_index;
-            exists = false;
             worksheetName = QStringLiteral("Sheet%1").arg(d->last_sheet_index);
-            for (int i=0; i<d->worksheets.size(); ++i) {
+        } while (d->worksheetNames.contains(worksheetName));
-                if (d->worksheets[i]->sheetName() == worksheetName)
-                    exists = true;
-            }
-        } while (exists);
     }
 
     ++d->last_sheet_id;
     Worksheet *sheet = new Worksheet(worksheetName, d->last_sheet_id, this);
     d->worksheets.insert(index, QSharedPointer<Worksheet>(sheet));
+    d->worksheetNames.insert(index, worksheetName);
     d->activesheetIndex = index;
     return sheet;
 }
@@ -242,6 +244,7 @@ bool Workbook::renameWorksheet(int index, const QString &name)
     }
 
     d->worksheets[index]->setSheetName(name);
+    d->worksheetNames[index] = name;
     return true;
 }
 
@@ -256,6 +259,7 @@ bool Workbook::deleteWorksheet(int index)
     if (index < 0 || index >= d->worksheets.size())
         return false;
     d->worksheets.removeAt(index);
+    d->worksheetNames.removeAt(index);
     return true;
 }
 
@@ -272,10 +276,14 @@ bool Workbook::moveWorksheet(int srcIndex, int distIndex)
         return false;
 
     QSharedPointer<Worksheet> sheet = d->worksheets.takeAt(srcIndex);
-    if (distIndex >= 0 || distIndex <= d->worksheets.size())
+    d->worksheetNames.takeAt(srcIndex);
+    if (distIndex >= 0 || distIndex <= d->worksheets.size()) {
         d->worksheets.insert(distIndex, sheet);
-    else
+        d->worksheetNames.insert(distIndex, sheet->sheetName());
+    } else {
         d->worksheets.append(sheet);
+        d->worksheetNames.append(sheet->sheetName());
+    }
     return true;
 }
 
@@ -288,28 +296,20 @@ bool Workbook::copyWorksheet(int index, const QString &newName)
     QString worksheetName = newName;
     if (!newName.isEmpty()) {
         //If user given an already in-used name, we should not continue any more!
-        for (int i=0; i<d->worksheets.size(); ++i) {
+        if (d->worksheetNames.contains(newName))
-            if (d->worksheets[i]->sheetName() == newName) {
+            return false;
-                return 0;
-            }
-        }
     } else {
         int copy_index = 1;
-        bool exists;
         do {
             ++copy_index;
-            exists = false;
             worksheetName = QStringLiteral("%1(%2)").arg(d->worksheets[index]->sheetName()).arg(copy_index);
-            for (int i=0; i<d->worksheets.size(); ++i) {
+        } while (d->worksheetNames.contains(worksheetName));
-                if (d->worksheets[i]->sheetName() == worksheetName)
-                    exists = true;
-            }
-        } while (exists);
     }
 
     ++d->last_sheet_id;
     QSharedPointer<Worksheet> sheet = d->worksheets[index]->copy(worksheetName, d->last_sheet_id);
     d->worksheets.append(sheet);
+    d->worksheetNames.append(sheet->sheetName());
 
     return false;
 }

src/xlsx/xlsxworkbook.h

@@ -93,6 +93,7 @@ private:
     QList<QImage> images();
     QList<Drawing *> drawings();
     void prepareDrawings();
+    QStringList worksheetNames() const;
     Worksheet *addWorksheet(const QString &name, int sheetId);
     WorkbookPrivate * const d_ptr;
 };

src/xlsx/xlsxworkbook_p.h

@@ -80,6 +80,7 @@ public:
 
     QSharedPointer<SharedStrings> sharedStrings;
     QList<QSharedPointer<Worksheet> > worksheets;
+    QStringList worksheetNames;
     QSharedPointer<Styles> styles;
     QSharedPointer<Theme> theme;
     QList<QImage> images;

src/xlsx/xlsxworksheet.h

@@ -126,7 +126,6 @@ public:
     void setWhiteSpaceVisible(bool visible);
 
     QString sheetName() const;
-    void setSheetName(const QString &sheetName);
 
     Workbook *workbook() const;
     ~Worksheet();
@@ -136,6 +135,7 @@ private:
     friend class ::WorksheetTest;
     Worksheet(const QString &sheetName, int sheetId, Workbook *book);
     QSharedPointer<Worksheet> copy(const QString &distName, int distId) const;
+    void setSheetName(const QString &sheetName);
 
     void saveToXmlFile(QIODevice *device);
     QByteArray saveToXmlData();