Author Topic: UFO (unidentified code)  (Read 9601 times)

Offline Alpha

  • Developer
  • Lives here!
  • *****
  • Posts: 1513
UFO (unidentified code)
« on: July 18, 2013, 02:01:27 am »
Reading through code, I noticed something strange.  In revision 9078
Code
http://cb.biplab.in/websvn/comp.php?repname=codeblocks&compare[]=/trunk/src/sdk/wxscintilla/src/scintilla/src/AutoComplete.cxx@9077&compare[]=/trunk/src/sdk/wxscintilla/src/scintilla/src/AutoComplete.cxx@9078
the piece of code
Code
diff --git a/src/sdk/wxscintilla/src/scintilla/src/AutoComplete.cxx b/src/sdk/wxscintilla/src/scintilla/src/AutoComplete.cxx
index 5f8cb8e..97a00dc 100644
--- a/src/sdk/wxscintilla/src/scintilla/src/AutoComplete.cxx
+++ b/src/sdk/wxscintilla/src/scintilla/src/AutoComplete.cxx
@@ -272,16 +272,6 @@ void AutoComplete::Select(const char *word) {
  break;
  }
  }
-/* C::B begin */
- // Check for a logically earlier match
- for (++pivot; pivot <= end; ++pivot) {
- lb->GetValue(sortMatrix[pivot], item, maxItemLen);
- if (CompareNCaseInsensitive(word, item, lenWord))
- break;
- if (sortMatrix[pivot] < sortMatrix[location] && !strncmp(word, item, lenWord))
- location = pivot;
- }
-/* C::B end */
  } else if (cond < 0) {
  end = pivot - 1;
  } else if (cond > 0) {
@@ -308,4 +298,3 @@ void AutoComplete::Select(const char *word) {
  lb->Select(sortMatrix[location]);
  }
 }
-
appeared.
I recognize that block, because I did write it at one point in time while communicating with a Scinitilla developer, but how did it end up committed to our repository?  The purpose of it was made obsolete by a block of code I wrote that is already included later in the file.
Is there any reason it should remain in our repository?

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5915
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: UFO (unidentified code)
« Reply #1 on: July 18, 2013, 07:21:24 am »
Note, rev9078 against rev9077. I think you like did a reverse compare (9077 against 9078?)
Code
 .../wxscintilla/src/scintilla/src/AutoComplete.cxx | 129 +++++++++++++++++++--
 1 file changed, 120 insertions(+), 9 deletions(-)

diff --git a/src/sdk/wxscintilla/src/scintilla/src/AutoComplete.cxx b/src/sdk/wxscintilla/src/scintilla/src/AutoComplete.cxx
index 07568d0..5f8cb8e 100644
--- a/src/sdk/wxscintilla/src/scintilla/src/AutoComplete.cxx
+++ b/src/sdk/wxscintilla/src/scintilla/src/AutoComplete.cxx
@@ -10,7 +10,9 @@
 #include <stdio.h>
 #include <assert.h>
 
+#include <algorithm>
 #include <string>
+#include <vector>
 
 #include "Platform.h"
 
@@ -36,7 +38,8 @@ AutoComplete::AutoComplete() :
  dropRestOfWord(false),
  ignoreCaseBehaviour(SC_CASEINSENSITIVEBEHAVIOUR_RESPECTCASE),
  widthLBDefault(100),
- heightLBDefault(100) {
+ heightLBDefault(100),
+ autoSort(SC_ORDER_PRESORTED) {
  lb = ListBox::Allocate();
  stopChars[0] = '\0';
  fillUpChars[0] = '\0';
@@ -103,8 +106,91 @@ char AutoComplete::GetTypesep() const {
  return typesep;
 }
 
+struct Sorter {
+ AutoComplete *ac;
+ const char *list;
+ std::vector<int> indices;
+
+ Sorter(AutoComplete *ac_, const char *list_) : ac(ac_), list(list_) {
+ int i = 0;
+ while (list[i]) {
+ indices.push_back(i); // word start
+ while (list[i] != ac->GetTypesep() && list[i] != ac->GetSeparator() && list[i])
+ ++i;
+ indices.push_back(i); // word end
+ if (list[i] == ac->GetTypesep()) {
+ while (list[i] != ac->GetSeparator() && list[i])
+ ++i;
+ }
+ if (list[i] == ac->GetSeparator()) {
+ ++i;
+ // preserve trailing separator as blank entry
+ if (!list[i]) {
+ indices.push_back(i);
+ indices.push_back(i);
+ }
+ }
+ }
+ indices.push_back(i); // index of last position
+ }
+
+ bool operator()(int a, int b) {
+ int lenA = indices[a * 2 + 1] - indices[a * 2];
+ int lenB = indices[b * 2 + 1] - indices[b * 2];
+ int len  = std::min(lenA, lenB);
+ int cmp;
+ if (ac->ignoreCase)
+ cmp = CompareNCaseInsensitive(list + indices[a * 2], list + indices[b * 2], len);
+ else
+ cmp = strncmp(list + indices[a * 2], list + indices[b * 2], len);
+ if (cmp == 0)
+ cmp = lenA - lenB;
+ return cmp < 0;
+ }
+};
+
 void AutoComplete::SetList(const char *list) {
- lb->SetList(list, separator, typesep);
+ if (autoSort == SC_ORDER_PRESORTED) {
+ lb->SetList(list, separator, typesep);
+ sortMatrix.clear();
+ for (int i = 0; i < lb->Length(); ++i)
+ sortMatrix.push_back(i);
+ return;
+ }
+
+ Sorter IndexSort(this, list);
+ sortMatrix.clear();
+ for (int i = 0; i < (int)IndexSort.indices.size() / 2; ++i)
+ sortMatrix.push_back(i);
+ std::sort(sortMatrix.begin(), sortMatrix.end(), IndexSort);
+ if (autoSort == SC_ORDER_CUSTOM || sortMatrix.size() < 2) {
+ lb->SetList(list, separator, typesep);
+ PLATFORM_ASSERT(lb->Length() == static_cast<int>(sortMatrix.size()));
+ return;
+ }
+
+ std::string sortedList;
+ char item[maxItemLen];
+ for (size_t i = 0; i < sortMatrix.size(); ++i) {
+ int wordLen = IndexSort.indices[sortMatrix[i] * 2 + 2] - IndexSort.indices[sortMatrix[i] * 2];
+ strncpy(item, list + IndexSort.indices[sortMatrix[i] * 2], wordLen);
+ if ((i+1) == sortMatrix.size()) {
+ // Last item so remove separator if present
+ if ((wordLen > 0) && (item[wordLen-1] == separator))
+ wordLen--;
+ } else {
+ // Item before last needs a separator
+ if ((wordLen == 0) || (item[wordLen-1] != separator)) {
+ item[wordLen] = separator;
+ wordLen++;
+ }
+ }
+ item[wordLen] = '\0';
+ sortedList += item;
+ }
+ for (int i = 0; i < (int)sortMatrix.size(); ++i)
+ sortMatrix[i] = i;
+ lb->SetList(sortedList.c_str(), separator, typesep);
 }
 
 int AutoComplete::GetSelection() const {
@@ -154,7 +240,7 @@ void AutoComplete::Select(const char *word) {
  while ((start <= end) && (location == -1)) { // Binary searching loop
  int pivot = (start + end) / 2;
  char item[maxItemLen];
- lb->GetValue(pivot, item, maxItemLen);
+ lb->GetValue(sortMatrix[pivot], item, maxItemLen);
  int cond;
  if (ignoreCase)
  cond = CompareNCaseInsensitive(word, item, lenWord);
@@ -163,7 +249,7 @@ void AutoComplete::Select(const char *word) {
  if (!cond) {
  // Find first match
  while (pivot > start) {
- lb->GetValue(pivot-1, item, maxItemLen);
+ lb->GetValue(sortMatrix[pivot-1], item, maxItemLen);
  if (ignoreCase)
  cond = CompareNCaseInsensitive(word, item, lenWord);
  else
@@ -177,7 +263,7 @@ void AutoComplete::Select(const char *word) {
  && ignoreCaseBehaviour == SC_CASEINSENSITIVEBEHAVIOUR_RESPECTCASE) {
  // Check for exact-case match
  for (; pivot <= end; pivot++) {
- lb->GetValue(pivot, item, maxItemLen);
+ lb->GetValue(sortMatrix[pivot], item, maxItemLen);
  if (!strncmp(word, item, lenWord)) {
  location = pivot;
  break;
@@ -186,15 +272,40 @@ void AutoComplete::Select(const char *word) {
  break;
  }
  }
+/* C::B begin */
+ // Check for a logically earlier match
+ for (++pivot; pivot <= end; ++pivot) {
+ lb->GetValue(sortMatrix[pivot], item, maxItemLen);
+ if (CompareNCaseInsensitive(word, item, lenWord))
+ break;
+ if (sortMatrix[pivot] < sortMatrix[location] && !strncmp(word, item, lenWord))
+ location = pivot;
+ }
+/* C::B end */
  } else if (cond < 0) {
  end = pivot - 1;
  } else if (cond > 0) {
  start = pivot + 1;
  }
  }
- if (location == -1 && autoHide)
- Cancel();
- else
- lb->Select(location);
+ if (location == -1) {
+ if (autoHide)
+ Cancel();
+ else
+ lb->Select(-1);
+ } else {
+ if (autoSort == SC_ORDER_CUSTOM) {
+ // Check for a logically earlier match
+ char item[maxItemLen];
+ for (int i = location + 1; i <= end; ++i) {
+ lb->GetValue(sortMatrix[i], item, maxItemLen);
+ if (CompareNCaseInsensitive(word, item, lenWord))
+ break;
+ if (sortMatrix[i] < sortMatrix[location] && !strncmp(word, item, lenWord))
+ location = i;
+ }
+ }
+ lb->Select(sortMatrix[location]);
+ }
 }
 

If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline MortenMacFly

  • Administrator
  • Lives here!
  • *****
  • Posts: 9694
Re: UFO (unidentified code)
« Reply #2 on: July 18, 2013, 08:44:40 am »
Is there any reason it should remain in our repository?
If its obsolete: No.
Compiler logging: Settings->Compiler & Debugger->tab "Other"->Compiler logging="Full command line"
C::B Manual: https://www.codeblocks.org/docs/main_codeblocks_en.html
C::B FAQ: https://wiki.codeblocks.org/index.php?title=FAQ

Offline thomas

  • Administrator
  • Lives here!
  • *****
  • Posts: 3979
Re: UFO (unidentified code)
« Reply #3 on: July 18, 2013, 09:54:40 am »
The combination of the words "pivot" and "compare" makes this look like QuickSort?
"We should forget about small efficiencies, say about 97% of the time: Premature quotation is the root of public humiliation."

Offline Alpha

  • Developer
  • Lives here!
  • *****
  • Posts: 1513
Re: UFO (unidentified code)
« Reply #4 on: July 21, 2013, 06:05:23 pm »
Note, rev9078 against rev9077. I think you like did a reverse compare (9077 against 9078?)
Yes, I was reversed :D.
If its obsolete: No.
Removing...
The combination of the words "pivot" and "compare" makes this look like QuickSort?
The section I quoted is actually part of a slightly custom modification on a binary search.  (It searches a sorted matrix of indices instead of the autocompletion list itself, allowing binary search to function on a non-alphabetical list.)