GList Anti-patterns
g_list_length(children);
for (int i = 0; i < (int)num; i++) {
GList * child = g_list_nth(children, num - i - 1);
if (g_list_length(nb_pages) != 0) {
for( i=0; i < g_list_length( GTK_CLIST(clist)->selection; i++ ){
gint row = (gint)g_list_nth_data( GTK_CLIST(clist)->selection, i);
Posted by mammadori at Thu Jul 16 17:19:40 2009:
Those are some really inefficient patterns, g_list_nth() and g_list_length() are O(n) functions.
Maybe it is time to use GPtrArray or GArray there ?
Posted by Ross at Thu Jul 16 17:32:34 2009:
Posted by Tester at Thu Jul 16 18:26:40 2009:
Maybe it is time to use GPtrArray or GArray there ?
Maybe the GList doc should mention their complexity.. It may be useful for first year CS students who haven't learned about linked lists yet. That said, maybe they wont understand O() notation anyway...
Posted by xurfa at Thu Jul 16 19:14:07 2009:
for( i=0; i < g_list_length( GTK_CLIST(clist)->selection; i++ )
.. something is missing here!
BTW, I answered "100" for the question "Add 10 and 10 (required)" and it deleted my post :-P
Posted by Gustavo Sverzut Barbieri at Thu Jul 16 19:43:51 2009:
.. something is missing here!
BTW, I answered "100" for the question "Add 10 and 10 (required)" and it deleted my post :-P
Ross, you should add stupid things like this to your list:
copy = NULL;
for (; l; l = l->next)
copy = g_slist_append(copy, l->data);
people really like to APPEND to GList and GSlist, but this is very costly. And this is not about novices, I keep seeing this in major Gnome components. Then when we suddenly see things that do not scale, that's why.
Thanks for your post.
Posted by Tommi Komulainen at Thu Jul 16 22:46:45 2009:
copy = NULL;
for (; l; l = l->next)
copy = g_slist_append(copy, l->data);
people really like to APPEND to GList and GSlist, but this is very costly. And this is not about novices, I keep seeing this in major Gnome components. Then when we suddenly see things that do not scale, that's why.
Thanks for your post.
My pet peeve is:
if (g_list_find(list, obj)) {
list = g_list_remove(list, obj);
// unref or otherwise do some obj clean up
Posted by Tom at Thu Jul 16 23:17:43 2009:
Posted by Colin Walters at Fri Jul 17 00:17:44 2009:
if (g_list_find(list, obj)) {
list = g_list_remove(list, obj);
// unref or otherwise do some obj clean up
This is why places need a code review policy, and having more experienced programmers glance over code can make a big difference.
Posted by Richard at Fri Jul 17 01:31:45 2009:
At least in the docs there's a note about g_list_append()'s inefficiency. I agree that more notes and comments on the efficiency of different APIs would be appreciated so we don't have to guess or dig every time we want to use something.
It's sad having to learn a lot about every API you use to use it correctly. Not that you shouldn't know what you're doing, but some of these functions are presented as "normal things to do", and there are many APIs we have to use and trust when writing software.
When I wrote my own list APIs in university, I tended to waste more memory by having properties for length that were updated on insertion, deletion, etc.
I suppose, as a novice, I wish it was harder to do things incorrectly.
Posted by tomás zerolo at Fri Jul 17 10:35:46 2009:
It's sad having to learn a lot about every API you use to use it correctly. Not that you shouldn't know what you're doing, but some of these functions are presented as "normal things to do", and there are many APIs we have to use and trust when writing software.
When I wrote my own list APIs in university, I tended to waste more memory by having properties for length that were updated on insertion, deletion, etc.
I suppose, as a novice, I wish it was harder to do things incorrectly.
