After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 624005 - CompareFunc poorly documented and confusing
CompareFunc poorly documented and confusing
Status: RESOLVED FIXED
Product: libgee
Classification: Platform
Component: general
0.5.x
Other Linux
: Normal normal
: 0.7
Assigned To: libgee-maint
libgee-maint
Depends on: 592993
Blocks: 679587
 
 
Reported: 2010-07-10 04:59 UTC by Adam B
Modified: 2012-08-06 01:32 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Add missing CompareDataFunc type arguments (7.96 KB, patch)
2012-07-23 20:36 UTC, Evan Nemerson
committed Details | Review

Description Adam B 2010-07-10 04:59:29 UTC
I struggled a bit trying to sort a Gee.List<int> in descending order.  CompareFunc is very un-vala-like because it takes two void pointers as parameters:

[http://www.valadoc.org/glib-2.0/GLib.CompareFunc.html]

    public int  CompareFunc (void* a, void* b)


The documentation needs to say what the pointers actually point to.  Your typical C programmers will assume that all you need to do is cast and dereference, but that will cause a segfault!  After printing the raw pointer values I saw that they in fact hold the real integer values directly:

  stdout.printf("a=%u b=%u\n", a, b);  //a=0x2 b=0x1

Using pointers to store non-pointer values!  In my estimation this will be very unexpected to 99% of the programmers using Vala and Gee.  Could generics be used instead?
Comment 1 Maciej (Matthew) Piechotka 2010-07-10 06:55:48 UTC
Please add dependency on bug #592993. It is vala limitation not ours.
Comment 2 Adam B 2010-07-10 07:35:46 UTC
If feature #592993 is going to take a long time I'd suggest improving the documentation too.
Comment 3 Maciej (Matthew) Piechotka 2010-07-10 07:38:37 UTC
Isn't it generated from GLib docs?
Comment 4 Christer Nissen 2010-07-27 08:15:03 UTC
Yes, its autogenerated.

Still, I agree that a helpful example would not be so bad. I remember struggling with this method myself for many hours before getting my code to work. :)
Comment 5 Evan Nemerson 2012-07-23 20:36:07 UTC
Created attachment 219533 [details] [review]
Add missing CompareDataFunc type arguments
Comment 6 Maciej (Matthew) Piechotka 2012-07-24 07:39:53 UTC
I wonder if something like:

  enum Ordering {
    LT = -1, EQ = 0, GT = 1
  }

 would not be better as result.
Comment 7 Evan Nemerson 2012-07-24 08:01:15 UTC
(In reply to comment #6)
> I wonder if something like:
> 
>   enum Ordering {
>     LT = -1, EQ = 0, GT = 1
>   }
> 
>  would not be better as result.

CompareDataFunc is in glib, not libgee, so obviously that would have to change.  More importantly, though, you lose compatibility with existing comparison functions.
Comment 8 Maciej (Matthew) Piechotka 2012-07-30 00:04:13 UTC
Review of attachment 219533 [details] [review]:

Looks good.

> CompareDataFunc is in glib, not libgee, so obviously that would have to change.
>  More importantly, though, you lose compatibility with existing comparison
> functions.

Ups. My fault.
Comment 9 Maciej (Matthew) Piechotka 2012-08-06 01:31:57 UTC
commit b3768e475a7d71af17daeb66b9151d258c4a687c
Author: Evan Nemerson <evan@coeus-group.com>
Date:   Mon Jul 23 13:32:58 2012 -0700

    Add missing CompareDataFunc type arguments, fixes bug 624005