[Tellico-users] Re: two patches for sorting

[Andrew Bennett]

Hi Robby,

> attached file 'sortmult.diff' loops through all the values, so that,
>

>
I added some unit tests and tweaked your patch just a bit, but just
> committed it. You're right about the ordering for empty strings. I'm not
> sure why I did it that way, so I did reverse it to be consistent with
> strings.
>

Thank you.  I'm glad you wrote the unit tests: I looked at the tests you
committed but I don't understand how they work.  I hope they weren't too
much trouble!

> order in the group view.  The attached file 'sortgroups.diff' is a bit
>
> I see what you mean. I think your approach works well enough. I'm going to
> poke at it a bit more myself, but I'll certainly fix that sorting in the
> group view.
>

Thank you again!
I saw that you've already committed the changes; I was glad to see that you
found a good way to re-use the field type checking.

There are quite a lot of compiler warnings for some of the old internal c
>
 libraries I use, like btparse and pilotdb. Occasionally I think about
> fixing
> them up, but since they're old and crusty anyways, it's not worth the
> effort.
>

Makes perfect sense.

use it. I develop on 64-bit opensuse, so I don't know if there's anything
>

Neat, that's what I run at work :)

Andrew

---
Drew Bennett
drewbenn at gmail.com


On Fri, Dec 3, 2010 at 12:25 PM, Robby Stephenson <robby at periapsis.org>wrote:

> Hi Andrew,
>
> On Sunday 21 November 2010, Andrew Bennett wrote:
> > I've noticed two minor issues when sorting numbers.  I've created
> > patches for both of them, although there are probably better ways of
> > fixing them.
>
> Cool, thanks for taking the time and effort to do so!
>
> > The first, and less important, is that when a number field has
> > multiple values, sorting is only performed on the first value.  The
> > attached file 'sortmult.diff' loops through all the values, so that,
> > for example, "1; 2" will always come before "1; 3".  There's not a lot
> > to this patch, although one comment in NumberComparison::compare() had
> > me a little confused:
> >   // by default, an empty string would get sorted before "1" because
> > toFloat() turns it into "0"
> >   // I want the empty strings to be at the end
> > This is different from how strings get sorted, where empty strings get
> > put at the beginning (because it is just a simple string compare); it
> > seemed odd that numbers and strings are being treated differently.
>
> I added some unit tests and tweaked your patch just a bit, but just
> committed it. You're right about the ordering for empty strings. I'm not
> sure why I did it that way, so I did reverse it to be consistent with
> strings.
>
> > The second, which I'd really like to see make it into Tellico in some
> > form, is that numbers are sorted in alphabetic, rather than numeric,
> > order in the group view.  The attached file 'sortgroups.diff' is a bit
> > more complex, and when sorting in the group view it checks the field
> > type and calls the field's custom sort function for certain types
> > (number, boolean, rating, date, and LCC).
>
> I see what you mean. I think your approach works well enough. I'm going to
> poke at it a bit more myself, but I'll certainly fix that sorting in the
> group view.
>
> > Finally, when I build for the first time, I see a whole lot of
> > warnings about deprecated functions.  It could be due to my
> > environment (Ubuntu 10.10, 64-bit, GNOME), and they didn't seem to
> > affect anything.  I didn't pay any attention to them, but wanted to
> > mention it in case you care (they disappear on subsequent builds, so
> > it's pretty easy to forget about them).
>
> There are quite a lot of compiler warnings for some of the old internal c
> libraries I use, like btparse and pilotdb. Occasionally I think about
> fixing
> them up, but since they're old and crusty anyways, it's not worth the
> effort.
>
> If there are deprecated warnings for anything else, it'd be worth fixing.
> Sometimes, with KDE I think, there will be a warning like that just because
> I'm using a header that includes a deprecated declaration even if I never
> use it. I develop on 64-bit opensuse, so I don't know if there's anything
> different about GNOME on Ubuntu that might show more deprecation warnings,
> though.
>
> Thanks again, and sorry for the late reply!
> Robby
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/tellico-users/attachments/20101205/d2e4a2bc/attachment.html>