From d65ff871487bb39d26e7af765ce3214f034be82a Mon Sep 17 00:00:00 2001 From: Chocobo1 Date: Sun, 4 Oct 2015 20:48:43 +0800 Subject: [PATCH] Replace `naturalSort()` with `naturalCompare()`. --- src/base/utils/string.cpp | 79 +++++--------------------- src/base/utils/string.h | 6 +- src/gui/addnewtorrentdialog.cpp | 2 +- src/gui/properties/peerlistsortmodel.h | 23 +++----- src/gui/rss/automatedrssdownloader.cpp | 2 +- src/gui/search/searchsortmodel.cpp | 21 +++---- src/gui/torrentcontentfiltermodel.cpp | 28 ++++----- src/gui/transferlistfilterswidget.cpp | 28 ++++----- src/gui/transferlistsortmodel.cpp | 42 ++++++++------ 9 files changed, 82 insertions(+), 149 deletions(-) diff --git a/src/base/utils/string.cpp b/src/base/utils/string.cpp index 046b33d7e..7d43b6284 100644 --- a/src/base/utils/string.cpp +++ b/src/base/utils/string.cpp @@ -44,68 +44,6 @@ std::string Utils::String::toStdString(const QString &str) return std::string(utf8.constData(), utf8.length()); } -// uses lessThan comparison -bool Utils::String::naturalSort(const QString &left, const QString &right, bool &result) -{ - // Return value indicates if functions was successful - // result argument will contain actual comparison result if function was successful - int posL = 0; - int posR = 0; - do { - forever { - if (posL == left.size() || posR == right.size()) - return false; // No data - - QChar leftChar = left.at(posL); - QChar rightChar = right.at(posR); - bool leftCharIsDigit = leftChar.isDigit(); - bool rightCharIsDigit = rightChar.isDigit(); - if (leftCharIsDigit != rightCharIsDigit) - return false; // Digit positions mismatch - - if (leftCharIsDigit) - break; // Both are digit, break this loop and compare numbers - - if (leftChar != rightChar) - return false; // Strings' subsets before digit do not match - - ++posL; - ++posR; - } - - QString temp; - while (posL < left.size()) { - if (left.at(posL).isDigit()) - temp += left.at(posL); - else - break; - posL++; - } - int numL = temp.toInt(); - temp.clear(); - - while (posR < right.size()) { - if (right.at(posR).isDigit()) - temp += right.at(posR); - else - break; - posR++; - } - int numR = temp.toInt(); - - if (numL != numR) { - result = (numL < numR); - return true; - } - - // Strings + digits do match and we haven't hit string end - // Do another round - - } while (true); - - return false; -} - Utils::String::NaturalCompare::NaturalCompare() { #if (QT_VERSION >= QT_VERSION_CHECK(5, 2, 0)) @@ -119,23 +57,25 @@ Utils::String::NaturalCompare::NaturalCompare() #endif } -bool Utils::String::NaturalCompare::operator()(const QString &l, const QString &r) +bool Utils::String::NaturalCompare::operator()(const QString &left, const QString &right) { + // case-insensitive comparison #if (QT_VERSION >= QT_VERSION_CHECK(5, 2, 0)) #if defined(Q_OS_WIN) // Without ICU library, QCollator doesn't support `setNumericMode(true)` on OS older than Win7 if(QSysInfo::windowsVersion() < QSysInfo::WV_WINDOWS7) - return lessThan(l, r); + return lessThan(left, right); #endif - return (m_collator.compare(l, r) < 0); + return (m_collator.compare(left, right) < 0); #else - return lessThan(l, r); + return lessThan(left, right); #endif } bool Utils::String::NaturalCompare::lessThan(const QString &left, const QString &right) { // Return value `false` indicates `right` should go before `left`, otherwise, after + // case-insensitive comparison int posL = 0; int posR = 0; while (true) { @@ -183,6 +123,13 @@ bool Utils::String::NaturalCompare::lessThan(const QString &left, const QString return false; } +bool Utils::String::naturalCompare(const QString &left, const QString &right) +{ + // provide a single `NaturalCompare` instance for easy use + static NaturalCompare nCmp; // this is thread-safe in C++11 (stated in spec 6.7.4) + return nCmp(left, right); +} + // to send numbers instead of strings with suffixes QString Utils::String::fromDouble(double n, int precision) { diff --git a/src/base/utils/string.h b/src/base/utils/string.h index 36b34e979..e379beadd 100644 --- a/src/base/utils/string.h +++ b/src/base/utils/string.h @@ -51,19 +51,19 @@ namespace Utils // Taken from https://crackstation.net/hashing-security.htm bool slowEquals(const QByteArray &a, const QByteArray &b); - bool naturalSort(const QString &left, const QString &right, bool &result); - class NaturalCompare { public: NaturalCompare(); - bool operator()(const QString &l, const QString &r); + bool operator()(const QString &left, const QString &right); bool lessThan(const QString &left, const QString &right); #if (QT_VERSION >= QT_VERSION_CHECK(5, 2, 0)) private: QCollator m_collator; #endif }; + + bool naturalCompare(const QString &left, const QString &right); } } diff --git a/src/gui/addnewtorrentdialog.cpp b/src/gui/addnewtorrentdialog.cpp index fa441c727..3511ef73f 100644 --- a/src/gui/addnewtorrentdialog.cpp +++ b/src/gui/addnewtorrentdialog.cpp @@ -96,7 +96,7 @@ AddNewTorrentDialog::AddNewTorrentDialog(QWidget *parent) // Load categories QStringList categories = session->categories(); - std::sort(categories.begin(), categories.end(), Utils::String::NaturalCompare()); + std::sort(categories.begin(), categories.end(), Utils::String::naturalCompare); QString defaultCategory = settings()->loadValue(KEY_DEFAULTCATEGORY).toString(); if (!defaultCategory.isEmpty()) diff --git a/src/gui/properties/peerlistsortmodel.h b/src/gui/properties/peerlistsortmodel.h index d37a9ccd1..ac298a3da 100644 --- a/src/gui/properties/peerlistsortmodel.h +++ b/src/gui/properties/peerlistsortmodel.h @@ -43,21 +43,16 @@ public: protected: bool lessThan(const QModelIndex &left, const QModelIndex &right) const { - if (sortColumn() == PeerListDelegate::IP || sortColumn() == PeerListDelegate::CLIENT) { - QVariant vL = sourceModel()->data(left); - QVariant vR = sourceModel()->data(right); - if (!(vL.isValid() && vR.isValid())) - return QSortFilterProxyModel::lessThan(left, right); - Q_ASSERT(vL.isValid()); - Q_ASSERT(vR.isValid()); - - bool res = false; - if (Utils::String::naturalSort(vL.toString(), vR.toString(), res)) - return res; + switch (sortColumn()) { + case PeerListDelegate::IP: + case PeerListDelegate::CLIENT: { + QString vL = left.data().toString(); + QString vR = right.data().toString(); + return Utils::String::naturalCompare(vL, vR); + } + }; - return QSortFilterProxyModel::lessThan(left, right); - } - return QSortFilterProxyModel::lessThan(left, right); + return QSortFilterProxyModel::lessThan(left, right); } }; diff --git a/src/gui/rss/automatedrssdownloader.cpp b/src/gui/rss/automatedrssdownloader.cpp index 7721462a7..744229710 100644 --- a/src/gui/rss/automatedrssdownloader.cpp +++ b/src/gui/rss/automatedrssdownloader.cpp @@ -314,7 +314,7 @@ void AutomatedRssDownloader::initCategoryCombobox() { // Load torrent categories QStringList categories = BitTorrent::Session::instance()->categories(); - std::sort(categories.begin(), categories.end(), Utils::String::NaturalCompare()); + std::sort(categories.begin(), categories.end(), Utils::String::naturalCompare); ui->comboCategory->addItems(categories); } diff --git a/src/gui/search/searchsortmodel.cpp b/src/gui/search/searchsortmodel.cpp index 22cc20fd6..0052029eb 100644 --- a/src/gui/search/searchsortmodel.cpp +++ b/src/gui/search/searchsortmodel.cpp @@ -107,20 +107,17 @@ qint64 SearchSortModel::maxSize() const bool SearchSortModel::lessThan(const QModelIndex &left, const QModelIndex &right) const { - if ((sortColumn() == NAME) || (sortColumn() == ENGINE_URL)) { - QVariant vL = sourceModel()->data(left); - QVariant vR = sourceModel()->data(right); - if (!(vL.isValid() && vR.isValid())) - return base::lessThan(left, right); - Q_ASSERT(vL.isValid()); - Q_ASSERT(vR.isValid()); - - bool res = false; - if (Utils::String::naturalSort(vL.toString(), vR.toString(), res)) - return res; + switch (sortColumn()) { + case NAME: + case ENGINE_URL: { + QString vL = left.data().toString(); + QString vR = right.data().toString(); + return Utils::String::naturalCompare(vL, vR); } - return base::lessThan(left, right); + default: + return base::lessThan(left, right); + }; } bool SearchSortModel::filterAcceptsRow(int sourceRow, const QModelIndex &sourceParent) const diff --git a/src/gui/torrentcontentfiltermodel.cpp b/src/gui/torrentcontentfiltermodel.cpp index ac0a0edf5..fb4d849b1 100644 --- a/src/gui/torrentcontentfiltermodel.cpp +++ b/src/gui/torrentcontentfiltermodel.cpp @@ -82,28 +82,22 @@ bool TorrentContentFilterModel::filterAcceptsRow(int source_row, const QModelInd } bool TorrentContentFilterModel::lessThan(const QModelIndex &left, const QModelIndex &right) const { - if (sortColumn() == NAME) { - QVariant vL = sourceModel()->data(left); - QVariant vR = sourceModel()->data(right); - if (!(vL.isValid() && vR.isValid())) - return QSortFilterProxyModel::lessThan(left, right); - Q_ASSERT(vL.isValid()); - Q_ASSERT(vR.isValid()); + switch (sortColumn()) { + case NAME: { // PropColumn::NAME + QString vL = left.data().toString(); + QString vR = right.data().toString(); + TorrentContentModelItem::ItemType leftType = m_model->itemType(m_model->index(left.row(), 0, left.parent())); + TorrentContentModelItem::ItemType rightType = m_model->itemType(m_model->index(right.row(), 0, right.parent())); - TorrentContentModelItem::ItemType leftType, rightType; - leftType = m_model->itemType(m_model->index(left.row(), 0, left.parent())); - rightType = m_model->itemType(m_model->index(right.row(), 0, right.parent())); - if (leftType == rightType) { - bool res = false; - if (Utils::String::naturalSort(vL.toString(), vR.toString(), res)) - return res; - return QSortFilterProxyModel::lessThan(left, right); - } + if (leftType == rightType) + return Utils::String::naturalCompare(vL, vR); else if (leftType == TorrentContentModelItem::FolderType && sortOrder() == Qt::AscendingOrder) return true; else - return false; + return false; } + }; + return QSortFilterProxyModel::lessThan(left, right); } diff --git a/src/gui/transferlistfilterswidget.cpp b/src/gui/transferlistfilterswidget.cpp index c83f48b87..a2da34141 100644 --- a/src/gui/transferlistfilterswidget.cpp +++ b/src/gui/transferlistfilterswidget.cpp @@ -237,17 +237,14 @@ void CategoryFiltersList::addItem(const QString &category, bool hasTorrent) if (exists) return; Q_ASSERT(count() >= 2); + int insPos = count(); for (int i = 2; i < count(); ++i) { - bool less = false; - if (!(Utils::String::naturalSort(category, item(i)->text(), less))) - less = (category.localeAwareCompare(item(i)->text()) < 0); - if (less) { - insertItem(i, categoryItem); - updateGeometry(); - return; + if (Utils::String::naturalCompare(category, item(i)->text())) { + insPos = i; + break; } } - QListWidget::addItem(categoryItem); + QListWidget::insertItem(insPos, categoryItem); updateGeometry(); } @@ -512,17 +509,14 @@ void TrackerFiltersList::addItem(const QString &tracker, const QString &hash) } Q_ASSERT(count() >= 4); - for (int i = 4; itext(), less))) - less = (host.localeAwareCompare(item(i)->text()) < 0); - if (less) { - insertItem(i, trackerItem); - updateGeometry(); - return; + int insPos = count(); + for (int i = 4; i < count(); ++i) { + if (Utils::String::naturalCompare(host, item(i)->text())) { + insPos = i; + break; } } - QListWidget::addItem(trackerItem); + QListWidget::insertItem(insPos, trackerItem); updateGeometry(); } diff --git a/src/gui/transferlistsortmodel.cpp b/src/gui/transferlistsortmodel.cpp index c69253666..06eeed220 100644 --- a/src/gui/transferlistsortmodel.cpp +++ b/src/gui/transferlistsortmodel.cpp @@ -73,21 +73,19 @@ void TransferListSortModel::disableTrackerFilter() bool TransferListSortModel::lessThan(const QModelIndex &left, const QModelIndex &right) const { - const int column = sortColumn(); - - if (column == TorrentModel::TR_NAME) { + switch (sortColumn()) { + case TorrentModel::TR_NAME: { QVariant vL = left.data(); QVariant vR = right.data(); if (!vL.isValid() || !vR.isValid() || (vL == vR)) return lowerPositionThan(left, right); - bool res = false; - if (Utils::String::naturalSort(vL.toString(), vR.toString(), res)) - return res; - - return QSortFilterProxyModel::lessThan(left, right); + return Utils::String::naturalCompare(vL.toString(), vR.toString()); } - else if (column == TorrentModel::TR_ADD_DATE || column == TorrentModel::TR_SEED_DATE || column == TorrentModel::TR_SEEN_COMPLETE_DATE) { + + case TorrentModel::TR_ADD_DATE: + case TorrentModel::TR_SEED_DATE: + case TorrentModel::TR_SEEN_COMPLETE_DATE: { QDateTime vL = left.data().toDateTime(); QDateTime vR = right.data().toDateTime(); @@ -97,10 +95,13 @@ bool TransferListSortModel::lessThan(const QModelIndex &left, const QModelIndex return vL < vR; } - else if (column == TorrentModel::TR_PRIORITY) { + + case TorrentModel::TR_PRIORITY: { return lowerPositionThan(left, right); } - else if (column == TorrentModel::TR_PEERS || column == TorrentModel::TR_SEEDS) { + + case TorrentModel::TR_SEEDS: + case TorrentModel::TR_PEERS: { int left_active = left.data().toInt(); int left_total = left.data(Qt::UserRole).toInt(); int right_active = right.data().toInt(); @@ -116,7 +117,8 @@ bool TransferListSortModel::lessThan(const QModelIndex &left, const QModelIndex return (left_active < right_active); } } - else if (column == TorrentModel::TR_ETA) { + + case TorrentModel::TR_ETA: { TorrentModel *model = qobject_cast(sourceModel()); const int prioL = model->data(model->index(left.row(), TorrentModel::TR_PRIORITY)).toInt(); const int prioR = model->data(model->index(right.row(), TorrentModel::TR_PRIORITY)).toInt(); @@ -164,7 +166,8 @@ bool TransferListSortModel::lessThan(const QModelIndex &left, const QModelIndex return !invalidL; } } - else if (column == TorrentModel::TR_LAST_ACTIVITY) { + + case TorrentModel::TR_LAST_ACTIVITY: { const qlonglong vL = left.data().toLongLong(); const qlonglong vR = right.data().toLongLong(); @@ -173,7 +176,8 @@ bool TransferListSortModel::lessThan(const QModelIndex &left, const QModelIndex return vL < vR; } - else if (column == TorrentModel::TR_RATIO_LIMIT) { + + case TorrentModel::TR_RATIO_LIMIT: { const qreal vL = left.data().toDouble(); const qreal vR = right.data().toDouble(); @@ -183,10 +187,12 @@ bool TransferListSortModel::lessThan(const QModelIndex &left, const QModelIndex return vL < vR; } - if (left.data() == right.data()) - return lowerPositionThan(left, right); - - return QSortFilterProxyModel::lessThan(left, right); + default: { + if (left.data() == right.data()) + return lowerPositionThan(left, right); + return QSortFilterProxyModel::lessThan(left, right); + } + }; } bool TransferListSortModel::lowerPositionThan(const QModelIndex &left, const QModelIndex &right) const