Bug 53662 - Add foreach support for libzeitgeist2's SimpleResultSet
Summary: Add foreach support for libzeitgeist2's SimpleResultSet
Status: RESOLVED FIXED
Alias: None
Product: Zeitgeist
Classification: Unclassified
Component: python-zeitgeist (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Seif Lotfy
QA Contact: zeitgeist-bugs@lists.freedesktop.org
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-08-17 23:04 UTC by Seif Lotfy
Modified: 2016-10-10 15:14 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
Add foreach support to ResultSet (3.54 KB, patch)
2012-08-18 12:49 UTC, Seif Lotfy
Details | Splinter Review
Add foreach support to ResultSet (4.21 KB, patch)
2012-08-18 13:41 UTC, Seif Lotfy
Details | Splinter Review
Add foreach support to ResultSet (3.26 KB, patch)
2012-08-18 14:17 UTC, Seif Lotfy
Details | Splinter Review
Add foreach support to ResultSet 2 (5.14 KB, patch)
2012-08-18 17:21 UTC, Seif Lotfy
Details | Splinter Review

Description Seif Lotfy 2012-08-17 23:04:27 UTC
We only need to implement three things
The collection class needs to implement an iterator() method that returns an object capable of iterating over your collection
The iterator object needs to provide two methods:
* bool next()
* T get () where T is your element type

---
vala will translate the foreach to something like
var iter = yourcollection.iterator ();
while (iter.next ()) {
var element = iter.get ();
... your foreach loop body ...
}
if the collection class supports that, it should work with foreach
Comment 1 Tom Beckmann 2012-08-17 23:57:57 UTC
If we'd just inherit SimpleResultSet from the underlying GLib.GenericArray instead of just Object, we'd get this and a lot more for free.
Comment 2 Michal Hruby 2012-08-18 10:05:35 UTC
SimpleResultSet is just an internal implementation of ResultSet, ResultSet needs to have the iterator interface, not SimpleResultSet.
Comment 3 Seif Lotfy 2012-08-18 12:49:13 UTC
Created attachment 65736 [details] [review]
Add foreach support to ResultSet
Comment 4 Seif Lotfy 2012-08-18 13:41:26 UTC
Created attachment 65738 [details] [review]
Add foreach support to ResultSet
Comment 5 Seif Lotfy 2012-08-18 14:17:22 UTC
Created attachment 65740 [details] [review]
Add foreach support to ResultSet
Comment 6 Michal Hruby 2012-08-18 14:31:39 UTC
Comment on attachment 65740 [details] [review]
Add foreach support to ResultSet

Review of attachment 65740 [details] [review]:
-----------------------------------------------------------------

Looking great, just one thing:

::: test/direct/datamodel-test.vala
@@ +25,5 @@
>  {
>      Test.init (ref argv);
>  
>      Test.add_func ("/Datamodel/MatchesTemplate/anything", matches_template_anything_test);
> +    Test.add_func ("/Datamodel/MatchesTemplate/foreach", foreach_test);

Please add one more test that runs foreach twice on the ResultSet instance.
Comment 7 Seif Lotfy 2012-08-18 17:21:17 UTC
Created attachment 65742 [details] [review]
Add foreach support to ResultSet 2

An alternative implementation
Comment 8 Seif Lotfy 2012-08-20 21:50:24 UTC
Going with the second solution congrats to all


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.