rewrite search function to use iterators

Until now search() works on an array like object. This works fine if the
whole array is searched. But if only a part is searched, this creates an
unnecessary copy.

src/models/disassemblymodel.cpp

@@ -244,7 +244,17 @@ void DisassemblyModel::find(const QString& search, Direction direction, int curr
 
     auto endReached = [this] { emit searchEndReached(); };
 
-    const int resultIndex = ::search(m_data.disassemblyLines, current, direction, searchFunc, endReached);
+    int resultIndex = 0;
+
+    if (direction == Direction::Forward) {
+        resultIndex = ::search(m_data.disassemblyLines.cbegin(), m_data.disassemblyLines.cend(),
+                               m_data.disassemblyLines.cbegin() + current, searchFunc, endReached);
+    } else {
+        resultIndex = m_data.disassemblyLines.size() - 1
+            - ::search(m_data.disassemblyLines.crbegin(), m_data.disassemblyLines.crend(),
+                       std::make_reverse_iterator(m_data.disassemblyLines.cbegin() + current + 1), searchFunc,
+                       endReached);
+    }
 
     if (resultIndex >= 0) {
         emit resultFound(createIndex(resultIndex, DisassemblyColumn));

src/models/disassemblymodel.h

@@ -21,8 +21,6 @@ class Definition;
 class Repository;
 }
 
-enum class Direction;
-
 class DisassemblyModel : public QAbstractTableModel
 {
     Q_OBJECT
@@ -71,13 +69,19 @@ class DisassemblyModel : public QAbstractTableModel
         SyntaxHighlightRole,
     };
 
+    enum class Direction
+    {
+        Forward,
+        Backward
+    };
 signals:
     void resultFound(QModelIndex index);
     void searchEndReached();
 
 public slots:
     void updateHighlighting(int line);
-    void find(const QString& search, Direction direction, int offset);
+
+    void find(const QString& search, DisassemblyModel::Direction direction, int offset);
     void scrollToLine(const QString& lineNumber);
 
 private:
@@ -87,3 +91,5 @@ public slots:
     int m_numTypes = 0;
     int m_highlightLine = 0;
 };
+
+Q_DECLARE_METATYPE(DisassemblyModel::Direction)

src/models/search.h

@@ -9,22 +9,16 @@
 
 #include <algorithm>
 #include <iterator>
-#include <QVector>
-
-enum class Direction
-{
-    Forward,
-    Backward
-};
 
 /** a search function that wrap around at the end
  * this function evaluates searchFunc starting from current and returns the offset from begin. In case end is reached,
  * it calls endReached and starts again at begin
  *
  * return: offset from begin
  * */
-template<typename iter, typename SearchFunc, typename EndReached>
-int search_impl(iter begin, iter end, iter current, SearchFunc searchFunc, EndReached endReached)
+
+template<typename it, typename SearchFunc, typename EndReached>
+int search(const it begin, const it end, const it current, SearchFunc searchFunc, EndReached endReached)
 {
     if (begin == end)
         return -1;
@@ -40,28 +34,9 @@ int search_impl(iter begin, iter end, iter current, SearchFunc searchFunc, EndRe
 
     found = std::find_if(begin, start, searchFunc);
 
-    if (found != end) {
+    if (found != start) {
         return std::distance(begin, found);
     }
 
     return -1;
 }
-
-// a wrapper around search_impl that returns the result index from begin
-template<typename Array, typename SearchFunc, typename EndReached>
-int search(const Array& array, int current, Direction direction, SearchFunc searchFunc, EndReached endReached)
-{
-    current = std::clamp(0, static_cast<int>(array.size()), current);
-    int resultIndex = 0;
-
-    if (direction == Direction::Forward) {
-        resultIndex = ::search_impl(array.begin(), array.end(), array.begin() + current, searchFunc, endReached);
-    } else {
-        resultIndex = ::search_impl(array.rbegin(), array.rend(),
-                                    std::make_reverse_iterator(array.begin() + current) - 1, searchFunc, endReached);
-        if (resultIndex != -1) {
-            resultIndex = array.size() - resultIndex - 1;
-        }
-    }
-    return resultIndex;
-}

src/models/sourcecodemodel.cpp

@@ -254,8 +254,18 @@ void SourceCodeModel::find(const QString& search, Direction direction, int curre
     auto searchFunc = [&search](const QString& line) { return line.indexOf(search, 0, Qt::CaseInsensitive) != -1; };
 
     auto endReached = [this] { emit searchEndReached(); };
-    qDebug() << m_lines.mid(m_startLine, m_numLines);
-    const int resultIndex = ::search(m_lines.mid(m_startLine, m_numLines), current, direction, searchFunc, endReached);
+    int resultIndex = 0;
+
+    if (direction == Direction::Forward) {
+        resultIndex = ::search(m_lines.cbegin() + m_startLine, m_lines.cbegin() + m_startLine + m_numLines,
+                               m_lines.cbegin() + m_startLine + current, searchFunc, endReached);
+    } else {
+        auto searchResultIndex =
+            ::search(std::make_reverse_iterator(m_lines.cbegin() + m_startLine + m_numLines),
+                     std::make_reverse_iterator(m_lines.cbegin() + m_startLine),
+                     std::make_reverse_iterator(m_lines.cbegin() + m_startLine + current + 1), searchFunc, endReached);
+        resultIndex = searchResultIndex == -1 ? -1 : m_numLines - 1 - searchResultIndex;
+    }
 
     if (resultIndex >= 0) {
         emit resultFound(createIndex(resultIndex + 1, SourceCodeColumn));

src/models/sourcecodemodel.h

@@ -20,8 +20,6 @@ class Repository;
 class Definition;
 }
 
-enum class Direction;
-
 Q_DECLARE_METATYPE(QTextLine)
 
 class SourceCodeModel : public QAbstractTableModel
@@ -65,6 +63,11 @@ class SourceCodeModel : public QAbstractTableModel
         FileLineRole,
     };
 
+    enum Direction
+    {
+        Forward,
+        Backward
+    };
 signals:
     void resultFound(QModelIndex index);
     void searchEndReached();
@@ -73,7 +76,7 @@ public slots:
     void updateHighlighting(int line);
     void setSysroot(const QString& sysroot);
 
-    void find(const QString& search, Direction direction, int current);
+    void find(const QString& search, SourceCodeModel::Direction direction, int current);
     void scrollToLine(const QString& lineNumber);
 
 private:
@@ -89,3 +92,5 @@ public slots:
     QStringList m_lines;
     int m_highlightLine = 0;
 };
+
+Q_DECLARE_METATYPE(SourceCodeModel::Direction)

src/resultsdisassemblypage.cpp

@@ -397,13 +397,24 @@ ResultsDisassemblyPage::ResultsDisassemblyPage(CostContextMenu* costContextMenu,
 
         auto searchNext = [model, edit, additionalRows, searchResultIndex] {
             const auto offset = searchResultIndex->isValid() ? searchResultIndex->row() - additionalRows : 0;
-            model->find(edit->text(), Direction::Forward, offset);
+            // TODO: c++20
+            // use templated lambda to get type
+
+            if constexpr (std::is_same_v<decltype(model), DisassemblyModel*>) {
+                model->find(edit->text(), DisassemblyModel::Direction::Forward, offset);
+            } else {
+                model->find(edit->text(), SourceCodeModel::Direction::Forward, offset);
+            }
         };
 
         auto searchPrev = [model, edit, additionalRows, searchResultIndex] {
             const auto offset = searchResultIndex->isValid() ? searchResultIndex->row() - additionalRows : 0;
 
-            model->find(edit->text(), Direction::Backward, offset);
+            if constexpr (std::is_same<decltype(model), DisassemblyModel*>::value) {
+                model->find(edit->text(), DisassemblyModel::Direction::Backward, offset);
+            } else {
+                model->find(edit->text(), SourceCodeModel::Direction::Backward, offset);
+            }
         };
 
         auto findNextAction = KStandardAction::findNext(this, searchNext, actions);

tests/modeltests/tst_search.cpp

@@ -23,10 +23,10 @@ private slots:
     {
         const std::array<int, 0> testArray = {};
 
-        QCOMPARE(search_impl(
+        QCOMPARE(search(
                      testArray.begin(), testArray.end(), testArray.begin(), [](int) { return false; }, [] {}),
                  -1);
-        QCOMPARE(search_impl(
+        QCOMPARE(search(
                      testArray.rbegin(), testArray.rend(), testArray.rbegin(), [](int) { return false; }, [] {}),
                  -1);
     }
@@ -36,11 +36,11 @@ private slots:
 
         int maxOffset = testArray.size() - 1;
         for (int offset = 0; offset < maxOffset; offset++) {
-            QCOMPARE(search_impl(
+            QCOMPARE(search(
                          testArray.begin(), testArray.end(), testArray.begin() + offset,
                          [](int num) { return num == 2; }, [] {}),
                      1);
-            QCOMPARE(search_impl(
+            QCOMPARE(search(
                          testArray.rbegin(), testArray.rend(), testArray.rbegin() + offset,
                          [](int num) { return num == 2; }, [] {}),
                      3);
@@ -52,7 +52,7 @@ private slots:
         const std::array<int, 5> testArray = {1, 2, 3, 4, 5};
         {
             bool endReached = false;
-            QCOMPARE(search_impl(
+            QCOMPARE(search(
                          testArray.begin(), testArray.end(), testArray.begin() + 1, [](int i) { return i == 1; },
                          [&endReached] { endReached = true; }),
                      0);
@@ -61,36 +61,21 @@ private slots:
 
         {
             bool endReached = false;
-            QCOMPARE(search_impl(
+            QCOMPARE(search(
                          testArray.rbegin(), testArray.rend(), std::make_reverse_iterator(testArray.begin() + 4 - 1),
                          [](int i) { return i == 4; }, [&endReached] { endReached = true; }),
                      1);
             QCOMPARE(endReached, true);
         }
     }
 
-    void testWrapper()
-    {
-        const std::array<int, 5> testArray = {1, 2, 3, 4, 5};
-
-        int maxOffset = testArray.size() - 1;
-        for (int i = 0; i < maxOffset; i++) {
-            QCOMPARE(search(
-                         testArray, i, Direction::Forward, [](int i) { return i == 4; }, [] {}),
-                     3);
-            QCOMPARE(search(
-                         testArray, i, Direction::Backward, [](int i) { return i == 4; }, [] {}),
-                     3);
-        }
-    }
-
     void testArrayIsEmpty()
     {
         const std::array<int, 0> testArray;
 
         for (int i = 0; i < 2; i++) {
             QCOMPARE(search(
-                         testArray, i, Direction::Forward, [](int) { return true; }, [] {}),
+                         testArray.cbegin(), testArray.cend(), testArray.cend(), [](int) { return true; }, [] {}),
                      -1);
         }
     }
@@ -100,9 +85,21 @@ private slots:
         const std::array<int, 1> testArray = {0};
 
         QCOMPARE(search(
-                     testArray, 1, Direction::Forward, [](int i) { return i == 0; }, [] {}),
+                     testArray.cbegin(), testArray.cend(), testArray.cbegin() + 1, [](int i) { return i == 0; }, [] {}),
                  0);
     }
+
+    void testSearchOnIterators()
+    {
+        const std::array<int, 5> testArray = {0, 1, 2, 3, 0};
+
+        for (int i = 1; i < 3; i++) {
+            QCOMPARE(search(
+                         testArray.cbegin() + 1, testArray.cbegin() + 3, testArray.cbegin() + i,
+                         [](int i) { return i == 0; }, [] {}),
+                     -1);
+        }
+    }
 };
 
 QTEST_GUILESS_MAIN(TestSearch)