[Octave-bug-tracker] [bug #51329] nth_element / median function fail with disable_range(true) (or with --traditional)

[Mike Miller]

Follow-up Comment #14, bug #51329 (project octave):

+1 on the timing test. Would be good to compare a range and the vector form of
the same range.

This patch looks better. I would recommend removing the extra "break"
statements, more than one way to exit from this case is confusing. I would
also make this case the last one, not sure if it makes a difference in
compiled code but it helps readability if the less complex cases come before
the more complex ones.

Something like this (untested) should help clean up the "break"s:


diff --git a/liboctave/array/Array.cc b/liboctave/array/Array.cc
--- a/liboctave/array/Array.cc
+++ b/liboctave/array/Array.cc
@@ -2337,6 +2337,30 @@ Array<T>::nth_element (const idx_vector&
       mode = ASCENDING;
       lo = n(0);
       break;
+    case idx_vector::class_vector:
+      if (n(1) - n(0) == 1)
+        {
+          mode = ASCENDING;
+          lo = n(0);
+        }
+      else if (n(1) - n(0) == -1)
+        {
+          mode = DESCENDING;
+          lo = ns - 1 - n(0);
+        }
+
+      for (int i = 2; (i < n.length()) && (mode != UNSORTED); ++i)
+        {
+          if ((mode == ASCENDING && n(i) - n(i-1) != 1) ||
+             (mode == DESCENDING && n(i) - n(i-1) != -1))
+            mode = UNSORTED;
+        }
+      break;
     case idx_vector::class_range:
       {
         octave_idx_type inc = n.increment ();


Some small additional cleanup with the indentation, line break should be
before the || operator, braces for the for loop are unnecessary, etc.

    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/bugs/?51329>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/