Replace `naturalSort()` with `naturalCompare()`.

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)
 {

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);
     }
 }
 

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())

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);
   }
 };
 

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);
 }
 

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

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);
 }
 

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; i<count(); ++i) {
-        bool less = false;
-        if (!(Utils::String::naturalSort(host, item(i)->text(), 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();
 }
 

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<TorrentModel *>(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