octave-maintainers
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: issorted & sortrows


From: John W. Eaton
Subject: Re: issorted & sortrows
Date: Thu, 12 Feb 2009 03:02:19 -0500

On 11-Feb-2009, David Bateman wrote:

| Jaroslav Hajek wrote:
| > On Wed, Feb 11, 2009 at 7:29 PM, John W. Eaton <address@hidden> wrote:
| >   
| >> On 11-Feb-2009, Jaroslav Hajek wrote:
| >>
| >> | Here, this is not so easily possible. The type of
| >> | octave_sort<T>::compare is bool (*) (T, T), not bool (*) (const T&,
| >> | const T&), so you'd get a type mismatch. The first form is probably
| >> | more suitable for simple built-in types, the latter for complex ones.
| >> | Supporting both would be possible but somewhat complicated.
| >>
| >> I don't see why it shouldn't use "const T&".  Doing that shouldn't
| >> cause trouble for built-in types, and has some advantage for aggregate
| >> types like Complex or some other class or structure that we may use
| >> later.  I don't understand why it was ever defined to just use T.
| >>
| >>     
| >
| > Apparently it was David who did it; presumably because he thought it'd
| > be faster for the simple types. Now, because for the simple types the
| > comparison is inlined anyway, even this vague reason is probably gone,
| > so I'm all for changing that to const T&, const T&, presumably
| > together with a typedef inside octave_sort (as suggested in the
| > following mail).
| >
| > If you  want to do it, please go ahead, otherwise I'll do it in near future.
| >   
| Hey its a few years back that I did this so all I can say is that you 
| are probably right on the reason..

OK.  I checked in the following changeset that uses some template
magic to allow us to use "T const&" for the parameters of these
functions if T is a class type, and to use "T" if T is not a class
type.  It also simplifies some declarations with typedefs and removes
the strange comparison functions for octave_value objects from the
src/TEMPLATE-INST/Array-tc.cc file.

  http://hg.savannah.gnu.org/hgweb/octave/rev/d5af326a3ede

jwe


reply via email to

[Prev in Thread] Current Thread [Next in Thread]