« Return to Thread: qx.ui.table.model.Simple.setSortMethods()

Re: qx.ui.table.model.Simple.setSortMethods()

by dmbaggett :: Rate this Message:

Reply to Author | View in Thread

I agree with Ian: swapping parameters to the user-provided sort-function should work just fine. JavaScript's Array sort method expects a function that imposes a "total order" on the objects to be sorted. In particular,

f(a, b) =
  -1 if a < b
   0 if a == b
   1 if a > b

By reversing the parameters, it's easy to see that you're just swapping the -1 and 1 values:

f(b, a) =
  -1 if b < a
   0 if b == a
   1 if b > a

So provided that function f() is correct from the standpoint of Array.sort, its inverse (obtained by swapping the a and b parameters) is also correct, and will sort in the opposite order.

The argument given in the source for having two methods is "performance reasons". But swapping parameters obviously causes no performance degradation.

For the new (to-be-named) enhanced table model which I recently submitted as an enhancement, this is a trivial change.

The only question is whether we want to do anything to avoid breaking existing code as this will constitute an API change.

Dave

Derrell Lipman wrote:
On Mon, Jul 6, 2009 at 6:01 AM, Ian Horst <ian.horst@googlemail.com> wrote:

> Derrell,
>
> I see your point. But let's take away from developers to define
> descending function. Qooxdoo framework can figure it out itself. Less
> code is better. :)
>
> I don't have time to look at the backend of qooxdoo right now, but
> instead of setSortMethods we could use
>
> ...setSortMethod: function (sortAscMethod)
> {
>  var sortDescMethod = function (row1, row2) {return
> sortAscMethod(row2, row1);};
>  var sortMethods = {ascending: sortAscMethod, descending: sortDescMethod};
>  ...
> }
>

Ian, that sounds like a great idea. I need to think about it some more to
ensure there aren't cases where simply swapping the parameters wouldn't do
the job, and it is an API change that should be well-considered. Would you
please create an enhancement bug for this, including the issue you're trying
to solve and your proposed solution. That way, this won't get forgotten.
It's likely not to get into the upcoming release, but can still make it into
0.9 if we agree it's appropriate.

Derrell

------------------------------------------------------------------------------

_______________________________________________
qooxdoo-devel mailing list
qooxdoo-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/qooxdoo-devel

 « Return to Thread: qx.ui.table.model.Simple.setSortMethods()