issorted & sortrows

[John W. Eaton]

On 11-Feb-2009, Jaroslav Hajek wrote:

| following the recent optimization of sort, I went ahead and
| implemented issorted and optimized sortrows:

Your patch has

  diff --git a/liboctave/Array-C.cc b/liboctave/Array-C.cc
  --- a/liboctave/Array-C.cc
  +++ b/liboctave/Array-C.cc
  @@ -28,41 +28,79 @@
   // Instantiate Arrays of Complex values.

   #include "oct-cmplx.h"
  +#include "lo-mappers.h"

   #include "Array.h"
   #include "Array.cc"
   #include "oct-sort.cc"

  -static double
  -xabs (const Complex& x)
  +template <>
  +inline bool _sort_isnan (Complex x)
   {
  -  return (xisinf (x.real ()) || xisinf (x.imag ())) ? octave_Inf : abs (x);
  +  return xisnan (x);
   }

Why the change from "const Complex&" to "Complex"?  That means you are
copying the Complex data structure on each call.  I think this should
still be a call by const reference.

For Octave, please also use "x" as a prefix for functions like this
instead of "_".

   template <>
   bool
   octave_sort<Complex>::ascending_compare (Complex a, Complex b)
   {
  -  return (xisnan (b) || (xabs (a) < xabs (b))
  -       || ((xabs (a) == xabs (b)) && (arg (a) < arg (b))));
  +  return ((std::abs (a) < std::abs (b))
  +       || ((std::abs (a) == std::abs (b)) && (arg (a) < arg (b))));
   }

Apparently this was already call by value?  I think these arguments
should also be "const Complex&", not "Complex".

  +static bool nan_ascending_compare (Complex x, Complex y)
  +{
  +  return xisnan (y) ? ! xisnan (x) : ((std::abs (x) < std::abs (x))
  +       || ((std::abs (x) == std::abs (x)) && (arg (x) < arg (x))));
  +}

For consistency with the rest of the Octave sources, and so that

  grep ^FCN_NAME

will find function definitions (but not uses), please write

  static bool
  nan_ascending_compare (const Complex& x, const Complex& y)


+bool (*_sortrows_comparator (sortmode mode, 
+                             const Array<Complex>& a , bool allow_chk)) 
+(Complex, Complex)
+{

This has me scratching my head.  Maybe a typedef would help here, so
you could write this as

  sortrows_comparator_function
  xsortrows_comparator (const Complex&, const Complex&)
  {

?  Or am I completely missing the point of this function signature?
If so, then I think a comment might help here, as this looks a little
obscure and is sure to confuse more people than just me.

jwe