(AUDIT?) Re: r14892 - gnucash/trunk - Add a new QOF_TYPE_NUMSTRING to add numeric sorts. (#150799).

[Derek Atkins]

Christian Stimming <stimming at tuhh.de> writes:

> Iff this change is okay for trunk, then IMHO it can very well be
> back-ported completely. However, are you sure the problem is solved in

Okay...  This is why I want the input..

> the optimum way by introducing a completely new QOF type? IMHO changes
> like these
>
>> Index: gnucash/trunk/lib/libqof/qof/qofquery.c
>> ===================================================================
>> --- gnucash/trunk/lib/libqof/qof/qofquery.c (revision 13747)
>> +++ gnucash/trunk/lib/libqof/qof/qofquery.c (revision 14892)
>> @@ -1694,5 +1694,6 @@
>>      return;
>>    }
>> -  if (!safe_strcmp (pd->type_name, QOF_TYPE_STRING))
>> +  if (!safe_strcmp (pd->type_name, QOF_TYPE_STRING) ||
>> +      !safe_strcmp (pd->type_name, QOF_TYPE_NUMSTRING))
>>    {
>>      query_string_t pdata = (query_string_t) pd;
>
> really suck. I readily admit I don't understand the QOF type system
> anyway, but from a naive understanding of the term "type" I would expect
> the sorting order *not* to be part of the definition of a type. And I
> would additionally predict that the next time someone needs to write
> strcmp(type_name, QOF_TYPE_STRING) somewhere, he/she will almost surely
> forget to additionally check for QOF_TYPE_NUMSTRING. Wasn't there any
> way to "only" change the sorting semantics as opposed to introducing a
> new type?

Well, here's the problem.  QOF is designed for objects (core types are
just "special" types of objects), and QOF defines object sort order on
a per-type (per-object-class) basis.  This means EVERYTHING you need
to know about how to sort on a particular parameter must be defined by
the type-name!  There's no "sorting argument" or "sorting predicate"
that you get to supply when defining sort order; all you get to define
is the parameter path which ends (effectively) in a type name.

Now...  We COULD go and add a "sort-by" override function to the QOF
Class parameter definitions, but that would be an even larger invasive
change because it would change the API to the QOF Class definition.
Perhaps that IS the right way to go if you're really afraid of these
kinds of changes?  But instead of only touching this specific type
definition it would require touching every single QOF Class
Parameter-list definition to add a new "column" to the table.  That
seems silly (on the face of it) to change a the sorting of single
parameter.  Then again it also seem silly to add a new type just for
one parameter, too, doesn't it?

So, I see two ways to approach this problem:

1) Add a new type, which keeps all the class definitions the same as
   they are (except changing TRANS_NUM from QOF_TYPE_STRING to
   QOF_TYPE_NUMSTRING).

2) Add a new (optional) QofParam member of type QofSortFunc, modify
   each and every place where we define QofParam (which is every
   QofClass definition everywhere in GnuCash), and change the code to
   prefer a QofParam QofSortFunc over the default type QofSortFunc.

I think both approaches are invasive.  The former is more "QOF API
friendly", the latter is probably "better" in that you can let each
parameter define its own sort function.

I'm happy to revert this change and work on the other one if you
prefer.

-derek

-- 
       Derek Atkins, SB '93 MIT EE, SM '95 MIT Media Laboratory
       Member, MIT Student Information Processing Board  (SIPB)
       URL: http://web.mit.edu/warlord/    PP-ASEL-IA     N1NWH
       warlord at MIT.EDU                        PGP key available