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

[Chris Shoemaker]

On Tue, Sep 26, 2006 at 08:16:20AM -0400, Derek Atkins wrote:
> 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.

What about...? 

3) Leave sorting out of the type-system altogether and sort only in
the view.  Save the sort preference in GConf.  (BTW, that's how it
works in all the GtkTreeModel/View pairs, including on the
register-rewrite branch.)

-chris


> 
> 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
> _______________________________________________
> gnucash-devel mailing list
> gnucash-devel at gnucash.org
> https://lists.gnucash.org/mailman/listinfo/gnucash-devel