zeitgeist team mailing list archive

[Siegfried Gevatter <rainct@xxxxxxxxxx>]

Siegfried Gevatter has proposed merging lp:~rainct/zeitgeist/delete-events-empty-array-exception into lp:zeitgeist.

Requested reviews:
  Zeitgeist Framework Team (zeitgeist)

For more details, see:
https://code.launchpad.net/~rainct/zeitgeist/delete-events-empty-array-exception/+merge/101390

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.
-- 
https://code.launchpad.net/~rainct/zeitgeist/delete-events-empty-array-exception/+merge/101390
Your team Zeitgeist Framework Team is requested to review the proposed merge of lp:~rainct/zeitgeist/delete-events-empty-array-exception into lp:zeitgeist.

=== modified file 'src/engine.vala'
--- src/engine.vala	2012-04-05 12:12:49 +0000
+++ src/engine.vala	2012-04-10 15:11:18 +0000
@@ -281,8 +281,13 @@
 
     public TimeRange? delete_events (uint32[] event_ids, BusName? sender)
         throws EngineError
-        requires (event_ids.length > 0)
     {
+        if (event_ids.length == 0)
+        {
+            throw new EngineError.INVALID_ARGUMENT (
+                "DeleteEvents: event_ids is empty");
+        }
+
         event_ids = extension_collection.call_pre_delete_events (
             event_ids, sender);