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 756988 - GSequence should document each function's complexity
GSequence should document each function's complexity
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: docs
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2015-10-22 20:09 UTC by Xavier Claessens
Modified: 2015-10-30 15:36 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
GSequence: document that g_sequence_get_length() is not O(1) (1.16 KB, patch)
2015-10-22 20:25 UTC, Xavier Claessens 		rejected Details | Review
GSequence: Stop using _get_length() to check if it's empty (854 bytes, patch)
2015-10-30 14:05 UTC, Xavier Claessens 		none Details | Review
GSequence: document that _get_length() is not O(1) (879 bytes, patch)
2015-10-30 14:06 UTC, Xavier Claessens 		none Details | Review
GSequence: document that _get_length() is not O(1) (879 bytes, patch)
2015-10-30 14:13 UTC, Xavier Claessens 		accepted-commit_now Details | Review
Stop using g_sequence_get_length() to check if it's empty (3.40 KB, patch)
2015-10-30 14:13 UTC, Xavier Claessens 		accepted-commit_now Details | Review

Description Xavier Claessens 2015-10-22 20:09:53 UTC 
Following bug #756316, I think it's important to document the O() of each method. Not even GSequence authors themself knew that _get_length is not O(1) judging from the while (g_sequence_get_length (tmp) > 0) I see in g_sequence_sort_iter().
Comment 1 Xavier Claessens 2015-10-22 20:25:41 UTC 
Created attachment 313892 [details] [review]
GSequence: document that g_sequence_get_length() is not O(1)
Comment 2 Allison Karlitskaya (desrt) 2015-10-30 10:15:32 UTC 
Review of attachment 313892 [details] [review]:

::: glib/gsequence.c
@@ +884,3 @@
   tmp->access_prohibited = TRUE;
 
+  while (!g_sequence_is_empty (tmp))

This looks like a documentation patch but it contains code changes.  Big no-no.

Please clarify the commit message or (ideally) split this up.
Comment 3 Xavier Claessens 2015-10-30 14:05:59 UTC 
Created attachment 314486 [details] [review]
GSequence: Stop using _get_length() to check if it's empty

g_sequence_is_empty() is more efficient for that task.
Comment 4 Xavier Claessens 2015-10-30 14:06:03 UTC 
Created attachment 314487 [details] [review]
GSequence: document that _get_length() is not O(1)
Comment 5 Xavier Claessens 2015-10-30 14:13:35 UTC 
Created attachment 314488 [details] [review]
GSequence: document that _get_length() is not O(1)
Comment 6 Xavier Claessens 2015-10-30 14:13:43 UTC 
Created attachment 314489 [details] [review]
Stop using g_sequence_get_length() to check if it's empty

g_sequence_is_empty() is more efficient for that task.
Comment 7 Xavier Claessens 2015-10-30 14:14:15 UTC 
A quick git grep gave many other places, fixed them as well now.
Comment 8 Allison Karlitskaya (desrt) 2015-10-30 15:24:30 UTC 
Review of attachment 314488 [details] [review]:

::: glib/gsequence.c
@@ +1232,3 @@
  * @seq: a #GSequence
  *
+ * Returns the length of @seq. Note that this method is O(h) where `h' is the

Put a paragraph break here (ie: your text is a new separate paragraph).

Otherwise, it's good.  Thanks.
Comment 9 Allison Karlitskaya (desrt) 2015-10-30 15:25:27 UTC 
Review of attachment 314489 [details] [review]:

OK.
Comment 10 Xavier Claessens 2015-10-30 15:36:08 UTC 
Thanks.