Created attachment 338550 [details] [review]
task-list-view: don't keep iterating unnecessarily

    When removing a task, we would iterate the entire list of rows, even
    continuing after we found the task and removed it. A task should not
    occur more than once in a list, so we can exit early once it's found.

Of course, due to https://bugzilla.gnome.org/show_bug.cgi?id=773554 it seems that the same task CAN appear in the list of row children more than once... but in an ideal world, that would not be possible!

So this would save time not scanning the rest of the list when there would be no purpose.

Created attachment 338551 [details] [review]
task-list: remove unneeded parentheses in return

    They do nothing, and we do not seem to have a convention of using them.

Created attachment 338552 [details] [review]
task-list-view: some code cleanups

    * remove_task(): merge the two ifs, don't redundantly call row_get_task
    * set_list   (): use if (find) instead of continue, to mirror above loop

I tried to restrain myself from attacking all the continue statements everywhere. :D

For these, I think they are tidier and more intuitive, but YMMV of course.

Review of attachment 338550 [details] [review]:

LGTM.

Review of attachment 338551 [details] [review]:

LGTM

(In reply to djb from comment #3)
> Created attachment 338552 [details] [review] [review]
> task-list-view: some code cleanups
> 
>     * remove_task(): merge the two ifs, don't redundantly call row_get_task
>     * set_list   (): use if (find) instead of continue, to mirror above loop
> 
> I tried to restrain myself from attacking all the continue statements
> everywhere. :D
> 
> For these, I think they are tidier and more intuitive, but YMMV of course.

buh, i forgot to invert the condition on the 2nd if... coming right up.

Review of attachment 338552 [details] [review]:

Needs work.

::: src/gtd-task-list-view.c
@@ +1613,3 @@
+                            G_CALLBACK (task_completed_cb),
+                            view);
+        }

I prefer to keep it the previous way, because it'd avoid getting too deep into nested calls. Using early breaks keeps the indentation low, and improves code readability.

Created attachment 338553 [details] [review]
task-list-view: apply a tiny cleanup/optimisation

    We have just verified that task == gtd_task_row_get_task (l->data), so
    we can use the former for shorter code and avoidance of a function call.

After reverting that patch to use continue; this is all that's left :P

Glad the others looked OK.

Review of attachment 338553 [details] [review]:

LGTM.

Thanks for all the patches! Code cleanups are always welcomed.

Attachment 338550 [details] pushed as 4aa0e24 - task-list-view: don't keep iterating unnecessarily
Attachment 338551 [details] pushed as 04f06e8 - task-list: remove unneeded parentheses in return
Attachment 338553 [details] pushed as 482822b - task-list-view: apply a tiny cleanup/optimisation