Merge lp:~rainct/zeitgeist/delete-events-empty-array-exception into lp:~zeitgeist/zeitgeist/bluebird

Proposed by Siegfried Gevatter
Status: Needs review
Proposed branch: lp:~rainct/zeitgeist/delete-events-empty-array-exception
Merge into: lp:~zeitgeist/zeitgeist/bluebird
Diff against target: 17 lines (+6/-1)
1 file modified
src/engine.vala (+6/-1)
To merge this branch: bzr merge lp:~rainct/zeitgeist/delete-events-empty-array-exception
Reviewer Review Type Date Requested Status
Zeitgeist Framework Team Pending
Review via email:

Description of the change

DeleteEvents currently requires a non-empty event_ids list, but this is checked with an assert, which is kinda ugly. I see two options here:

 1. Promote the assert to a proper INVALID_ARGUMENT exception.
 2. Instead decide to handle empty lists by just doing nothing and returning TimeRange(0, 0).

This branch implements the 1st behaviour. I don't really like the second option since:
 a) Returning (0,0) -or any other duplicate value- seems rather arbitrary, and it's a special case they'll have to check for if they want to use either early_timestamp or late_timestamp individualy. From the Zen of Python: Ā«Special cases aren't special enough to break the rulesĀ» :).
 b) It's trivial for applications to check what they are doing before wasting D-Bus and Zeitgeist's time.

To post a comment you must log in.

Unmerged revisions

468. By Siegfried Gevatter

DeleteEvents: promote assert to exception

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/engine.vala'
2--- src/engine.vala 2012-04-05 12:12:49 +0000
3+++ src/engine.vala 2012-04-10 15:11:18 +0000
4@@ -281,8 +281,13 @@
6 public TimeRange? delete_events (uint32[] event_ids, BusName? sender)
7 throws EngineError
8- requires (event_ids.length > 0)
9 {
10+ if (event_ids.length == 0)
11+ {
12+ throw new EngineError.INVALID_ARGUMENT (
13+ "DeleteEvents: event_ids is empty");
14+ }
16 event_ids = extension_collection.call_pre_delete_events (
17 event_ids, sender);


People subscribed via source and target branches