From 62af03e5111625a42fa1e4e3ace4c7ecd3fd0e64 Mon Sep 17 00:00:00 2001 From: Charles Lindsay Date: Thu, 12 Dec 2013 12:42:02 -0800 Subject: [PATCH] Add API to make juggling Gee collections easier This adds a simple Iterable class that lets us take advantage of Gee's Traversable interface much more easily. Traversable is great, but every operation returns an Iterator, which makes it awkward to use outside of Traversable. The new Iterable wraps the Traversable Iterators and methods so you can directly use the result. It also gives us a convenient point to add convenience methods in the future. I've gone through a few arbitrary places in the code to see how the class might be used, and changed some obvious places to (hopefully) the equivalent code using the new Iterable class. More work could be done here, but the real benefit is simply having the Iterable class around to be able to use in new code. --- src/CMakeLists.txt | 1 + src/client/application/geary-controller.vala | 8 +- src/engine/api/geary-named-flags.vala | 29 +--- src/engine/api/geary-search-folder.vala | 56 +++---- src/engine/app/app-conversation-monitor.vala | 25 ++-- .../app-conversation-set.vala | 12 +- src/engine/imap-db/imap-db-folder.vala | 10 +- .../imap-engine-generic-account.vala | 41 +++-- src/engine/imap/message/imap-flags.vala | 7 +- src/engine/util/util-iterable.vala | 141 ++++++++++++++++++ 10 files changed, 212 insertions(+), 118 deletions(-) create mode 100644 src/engine/util/util-iterable.vala diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 88290b89..3e1d60be 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -277,6 +277,7 @@ engine/util/util-generic-capabilities.vala engine/util/util-html.vala engine/util/util-imap-utf7.vala engine/util/util-inet.vala +engine/util/util-iterable.vala engine/util/util-numeric.vala engine/util/util-object.vala engine/util/util-reference-semantics.vala diff --git a/src/client/application/geary-controller.vala b/src/client/application/geary-controller.vala index 944dbbfa..cd600c2b 100644 --- a/src/client/application/geary-controller.vala +++ b/src/client/application/geary-controller.vala @@ -2003,11 +2003,9 @@ public class GearyController : Geary.BaseObject { // Returns a list of composer windows for an account, or null if none. public Gee.List? get_composer_windows_for_account(Geary.AccountInformation account) { - Gee.List ret = new Gee.LinkedList(); - foreach (ComposerWindow cw in composer_windows) { - if (cw.account.information == account) - ret.add(cw); - } + Gee.LinkedList ret = Geary.traverse(composer_windows) + .filter(w => w.account.information == account) + .to_linked_list(); return ret.size >= 1 ? ret : null; } diff --git a/src/engine/api/geary-named-flags.vala b/src/engine/api/geary-named-flags.vala index f3c069c2..ed4e93e3 100644 --- a/src/engine/api/geary-named-flags.vala +++ b/src/engine/api/geary-named-flags.vala @@ -34,11 +34,7 @@ public class Geary.NamedFlags : BaseObject, Gee.Hashable { } public bool contains_any(NamedFlags flags) { - foreach(NamedFlag nf in list) - if (flags.contains(nf)) - return true; - - return false; + return Geary.traverse(list).any(f => flags.contains(f)); } public Gee.Set get_all() { @@ -53,11 +49,9 @@ public class Geary.NamedFlags : BaseObject, Gee.Hashable { } public virtual void add_all(NamedFlags flags) { - Gee.ArrayList added = new Gee.ArrayList(); - foreach (NamedFlag flag in flags.get_all()) { - if (!list.contains(flag)) - added.add(flag); - } + Gee.ArrayList added = Geary.traverse(flags.get_all()) + .filter(f => !list.contains(f)) + .to_array_list(); list.add_all(added); notify_added(added); @@ -72,11 +66,9 @@ public class Geary.NamedFlags : BaseObject, Gee.Hashable { } public virtual bool remove_all(NamedFlags flags) { - Gee.ArrayList removed = new Gee.ArrayList(); - foreach (NamedFlag flag in flags.get_all()) { - if (list.contains(flag)) - removed.add(flag); - } + Gee.ArrayList removed = Geary.traverse(flags.get_all()) + .filter(f => list.contains(f)) + .to_array_list(); list.remove_all(removed); notify_removed(removed); @@ -91,12 +83,7 @@ public class Geary.NamedFlags : BaseObject, Gee.Hashable { if (list.size != other.list.size) return false; - foreach (NamedFlag flag in list) { - if (!other.contains(flag)) - return false; - } - - return true; + return Geary.traverse(list).all(f => other.contains(f)); } public uint hash() { diff --git a/src/engine/api/geary-search-folder.vala b/src/engine/api/geary-search-folder.vala index f2db844e..499d0a41 100644 --- a/src/engine/api/geary-search-folder.vala +++ b/src/engine/api/geary-search-folder.vala @@ -88,11 +88,10 @@ public class Geary.SearchFolder : Geary.AbstractLocalFolder, Geary.FolderSupport private void on_folders_available_unavailable(Gee.Collection? available, Gee.Collection? unavailable) { if (available != null) { - foreach (Geary.Folder folder in available) { - // Exclude it from searching if it's got the right special type. - if (folder.special_folder_type in exclude_types) - exclude_folder(folder); - } + // Exclude it from searching if it's got the right special type. + foreach(Geary.Folder folder in Geary.traverse(available) + .filter(f => f.special_folder_type in exclude_types)) + exclude_folder(folder); } } @@ -101,14 +100,12 @@ public class Geary.SearchFolder : Geary.AbstractLocalFolder, Geary.FolderSupport Cancellable? cancellable = null) throws Error { low = null; high = null; - Gee.TreeSet in_folder - = new Gee.TreeSet(); - foreach (Geary.EmailIdentifier id in ids) { - ImapDB.SearchEmailIdentifier? search_id = id as ImapDB.SearchEmailIdentifier; - // This shouldn't require a result_mutex lock since there's no yield. - if (search_id != null && search_id in search_results) - in_folder.add(search_id); - } + + // This shouldn't require a result_mutex lock since there's no yield. + Gee.TreeSet in_folder = Geary.traverse(ids) + .cast_object() + .filter(id => id in search_results) + .to_tree_set(); if (in_folder.size > 0) { low = in_folder.first(); @@ -154,13 +151,10 @@ public class Geary.SearchFolder : Geary.AbstractLocalFolder, Geary.FolderSupport Error? error = null; try { Gee.ArrayList relevant_ids - = new Gee.ArrayList(); - foreach (Geary.EmailIdentifier id in ids) { - ImapDB.SearchEmailIdentifier? search_id - = ImapDB.SearchEmailIdentifier.collection_get_email_identifier(search_results, id); - if (search_id != null) - relevant_ids.add(search_id); - } + = Geary.traverse(ids) + .map_nonnull( + id => ImapDB.SearchEmailIdentifier.collection_get_email_identifier(search_results, id)) + .to_array_list(); if (relevant_ids.size > 0) yield do_search_async(query, null, relevant_ids, cancellable); @@ -259,22 +253,20 @@ public class Geary.SearchFolder : Geary.AbstractLocalFolder, Geary.FolderSupport = ImapDB.SearchEmailIdentifier.array_list_from_results(yield account.local_search_async( query, MAX_RESULT_EMAILS, 0, exclude_folders, add_ids ?? remove_ids, cancellable)); - Gee.ArrayList added - = new Gee.ArrayList(); - Gee.ArrayList removed - = new Gee.ArrayList(); + Gee.List added + = Gee.List.empty(); + Gee.List removed + = Gee.List.empty(); if (remove_ids == null) { - foreach (ImapDB.SearchEmailIdentifier id in results) { - if (!(id in search_results)) - added.add(id); - } + added = Geary.traverse(results) + .filter(id => !(id in search_results)) + .to_array_list(); } if (add_ids == null) { - foreach (ImapDB.SearchEmailIdentifier id in remove_ids ?? search_results) { - if (!(id in results)) - removed.add(id); - } + removed = Geary.traverse(remove_ids ?? search_results) + .filter(id => !(id in results)) + .to_array_list(); } search_results.remove_all(removed); diff --git a/src/engine/app/app-conversation-monitor.vala b/src/engine/app/app-conversation-monitor.vala index 55e20a67..4eafbfde 100644 --- a/src/engine/app/app-conversation-monitor.vala +++ b/src/engine/app/app-conversation-monitor.vala @@ -417,14 +417,9 @@ public class Geary.App.ConversationMonitor : BaseObject { Gee.HashSet relevant_ids = new Gee.HashSet(); foreach (Geary.Email email in emails) { Gee.Set? ancestors = email.get_ancestors(); - if (ancestors != null) { - foreach (RFC822.MessageID ancestor in ancestors) { - if (conversations.has_message_id(ancestor)) { - relevant_ids.add(email.id); - break; - } - } - } + if (ancestors != null && + Geary.traverse(ancestors).any(id => conversations.has_message_id(id))) + relevant_ids.add(email.id); } debug("%d external emails are relevant to current conversations", relevant_ids.size); @@ -485,10 +480,9 @@ public class Geary.App.ConversationMonitor : BaseObject { Gee.Set? ancestors = email.get_ancestors(); if (ancestors != null) { - foreach (RFC822.MessageID ancestor in ancestors) { - if (!new_message_ids.contains(ancestor)) - new_message_ids.add(ancestor); - } + Geary.traverse(ancestors) + .filter(id => !new_message_ids.contains(id)) + .add_all_to(new_message_ids); } } } @@ -573,10 +567,9 @@ public class Geary.App.ConversationMonitor : BaseObject { foreach (int id in batch.get_ids()) { LocalSearchOperation op = (LocalSearchOperation) batch.get_operation(id); if (op.emails != null) { - foreach (Geary.Email email in op.emails.get_keys()) { - if (!needed_messages.has_key(email.id)) - needed_messages.set(email.id, email); - } + Geary.traverse(op.emails.get_keys()) + .filter(e => !needed_messages.has_key(e.id)) + .add_all_to_map(needed_messages, e => e.id); } } diff --git a/src/engine/app/conversation-monitor/app-conversation-set.vala b/src/engine/app/conversation-monitor/app-conversation-set.vala index 2246ffc9..06068d36 100644 --- a/src/engine/app/conversation-monitor/app-conversation-set.vala +++ b/src/engine/app/conversation-monitor/app-conversation-set.vala @@ -64,18 +64,14 @@ private class Geary.App.ConversationSet : BaseObject { // the ancestors of the supplied Email ... if more than one, then add_email() should not be // called private Gee.Set get_associated_conversations(Geary.Email email) { - Gee.Set associated = new Gee.HashSet(); - Gee.Set? ancestors = email.get_ancestors(); if (ancestors != null) { - foreach (Geary.RFC822.MessageID ancestor in ancestors) { - Conversation conversation = logical_message_id_map.get(ancestor); - if (conversation != null) - associated.add(conversation); - } + return Geary.traverse(ancestors) + .map_nonnull(a => logical_message_id_map.get(a)) + .to_hash_set(); } - return associated; + return Gee.Set.empty(); } /** diff --git a/src/engine/imap-db/imap-db-folder.vala b/src/engine/imap-db/imap-db-folder.vala index d9b6bc86..9bbc11f5 100644 --- a/src/engine/imap-db/imap-db-folder.vala +++ b/src/engine/imap-db/imap-db-folder.vala @@ -2070,19 +2070,15 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics { private int do_get_unread_count_for_ids(Db.Connection cx, Gee.Collection ids, Cancellable? cancellable) throws Error { - int unread_count = 0; - // Fetch flags for each email and update this folder's unread count. // (Note that this only flags for emails which have NOT been marked for removal // are included.) Gee.Map? flag_map = do_get_email_flags(cx, ids, cancellable); - if (flag_map != null) { - foreach (Geary.EmailFlags flags in flag_map.values) - unread_count += flags.is_unread() ? 1 : 0; - } + if (flag_map != null) + return Geary.traverse(flag_map.values).count_matching(f => f.is_unread()); - return unread_count; + return 0; } } diff --git a/src/engine/imap-engine/imap-engine-generic-account.vala b/src/engine/imap-engine/imap-engine-generic-account.vala index 66740f4f..d62f6399 100644 --- a/src/engine/imap-engine/imap-engine-generic-account.vala +++ b/src/engine/imap-engine/imap-engine-generic-account.vala @@ -241,16 +241,14 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.AbstractAccount { throws Error { check_open(); - Gee.ArrayList matches = new Gee.ArrayList(); - - foreach(FolderPath path in folder_map.keys) { - FolderPath? path_parent = path.get_parent(); - if ((parent == null && path_parent == null) || - (parent != null && path_parent != null && path_parent.equal_to(parent))) { - matches.add(folder_map.get(path)); - } - } - return matches; + return Geary.traverse(folder_map.keys) + .filter(p => { + FolderPath? path_parent = p.get_parent(); + return ((parent == null && path_parent == null) || + (parent != null && path_parent != null && path_parent.equal_to(parent))); + }) + .map(p => folder_map.get(p)) + .to_array_list(); } public override Gee.Collection list_folders() throws Error { @@ -316,9 +314,8 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.AbstractAccount { existing_list.add_all(build_folders(local_children.values)); existing_list.add_all(local_only.values); - Gee.HashMap existing_folders = new Gee.HashMap(); - foreach (Geary.Folder folder in existing_list) - existing_folders.set(folder.path, folder); + Gee.HashMap existing_folders + = Geary.traverse(existing_list).to_hash_map(f => f.path); // get all remote (server) folder paths Gee.HashMap remote_folders = yield enumerate_remote_folders_async(null, @@ -467,18 +464,16 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.AbstractAccount { } // If path in remote but not local, need to add it - Gee.List? to_add = new Gee.ArrayList(); - foreach (Imap.Folder remote_folder in remote_folders.values) { - if (!existing_folders.has_key(remote_folder.path)) - to_add.add(remote_folder); - } + Gee.ArrayList to_add = Geary.traverse(remote_folders.values) + .filter(f => !existing_folders.has_key(f.path)) + .to_array_list(); // If path in local but not remote (and isn't local-only, i.e. the Outbox), need to remove it - Gee.List? to_remove = new Gee.ArrayList(); - foreach (Geary.FolderPath existing_path in existing_folders.keys) { - if (!remote_folders.has_key(existing_path) && !local_only.has_key(existing_path)) - to_remove.add(existing_folders.get(existing_path)); - } + Gee.ArrayList to_remove + = Geary.traverse>(existing_folders) + .filter(e => !remote_folders.has_key(e.key) && !local_only.has_key(e.key)) + .map(e => (Geary.Folder) e.value) + .to_array_list(); // For folders to add, clone them and their properties locally foreach (Geary.Imap.Folder remote_folder in to_add) { diff --git a/src/engine/imap/message/imap-flags.vala b/src/engine/imap/message/imap-flags.vala index 42f09977..303419f4 100644 --- a/src/engine/imap/message/imap-flags.vala +++ b/src/engine/imap/message/imap-flags.vala @@ -55,12 +55,7 @@ public abstract class Geary.Imap.Flags : Geary.MessageData.AbstractMessageData, if (other.size != size) return false; - foreach (Flag flag in list) { - if (!other.contains(flag)) - return false; - } - - return true; + return Geary.traverse(list).all(f => other.contains(f)); } public override string to_string() { diff --git a/src/engine/util/util-iterable.vala b/src/engine/util/util-iterable.vala new file mode 100644 index 00000000..4c5ad058 --- /dev/null +++ b/src/engine/util/util-iterable.vala @@ -0,0 +1,141 @@ +/* Copyright 2013 Yorba Foundation + * + * This software is licensed under the GNU Lesser General Public License + * (version 2.1 or later). See the COPYING file in this distribution. + */ + +namespace Geary { + public Geary.Iterable traverse(Gee.Iterable i) { + return new Geary.Iterable(i.iterator()); + } +} + +/** + * An Iterable that simply wraps an existing Iterator. You get one iteration, + * and only one iteration. Basically every method triggers one iteration and + * returns a new object. + * + * Note that this can't inherit from Gee.Iterable because its interface + * requires that map/filter/etc. return Iterators, not Iterables. It still + * works in foreach. + */ + +public class Geary.Iterable : BaseObject { + private Gee.Iterator i; + + public Iterable(Gee.Iterator iterator) { + i = iterator; + } + + public virtual Gee.Iterator iterator() { + return i; + } + + public Iterable map(Gee.MapFunc f) { + return new Iterable(i.map(f)); + } + + public Iterable scan(Gee.FoldFunc f, owned A seed) { + return new Iterable(i.scan(f, seed)); + } + + public Iterable filter(owned Gee.Predicate f) { + return new Iterable(i.filter(f)); + } + + public Iterable chop(int offset, int length = -1) { + return new Iterable(i.chop(offset, length)); + } + + public Iterable map_nonnull(Gee.MapFunc f) { + return new Iterable(i.map(f).filter(g => g != null)); + } + + /** + * Return only objects of the destination type, as the destination type. + * Only works on types derived from Object. + */ + public Iterable cast_object() { + return new Iterable( + // This would be a lot simpler if valac didn't barf on the shorter, + // more obvious syntax for each of these delegates here. + i.filter(g => ((Object) g).get_type().is_a(typeof(A))) + .map(g => { return (A) g; })); + } + + public G? first() { + return (i.next() ? i.@get() : null); + } + + public G? first_matching(owned Gee.Predicate f) { + foreach (G g in this) { + if (f(g)) + return g; + } + return null; + } + + public bool any(owned Gee.Predicate f) { + foreach (G g in this) { + if (f(g)) + return true; + } + return false; + } + + public bool all(owned Gee.Predicate f) { + foreach (G g in this) { + if (!f(g)) + return false; + } + return true; + } + + public int count_matching(owned Gee.Predicate f) { + int count = 0; + foreach (G g in this) { + if (f(g)) + count++; + } + return count; + } + + public Gee.Collection add_all_to(Gee.Collection c) { + while (i.next()) + c.add(i.@get()); + return c; + } + + public Gee.ArrayList to_array_list(owned Gee.EqualDataFunc? equal_func = null) { + return (Gee.ArrayList) add_all_to(new Gee.ArrayList(equal_func)); + } + + public Gee.LinkedList to_linked_list(owned Gee.EqualDataFunc? equal_func = null) { + return (Gee.LinkedList) add_all_to(new Gee.LinkedList(equal_func)); + } + + public Gee.HashSet to_hash_set(owned Gee.HashDataFunc? hash_func = null, + owned Gee.EqualDataFunc? equal_func = null) { + return (Gee.HashSet) add_all_to(new Gee.HashSet(hash_func, equal_func)); + } + + public Gee.TreeSet to_tree_set(owned CompareDataFunc? compare_func = null) { + return (Gee.TreeSet) add_all_to(new Gee.TreeSet(compare_func)); + } + + public Gee.Map add_all_to_map(Gee.Map c, Gee.MapFunc key_func) { + while (i.next()) { + G g = i.@get(); + c.@set(key_func(g), g); + } + return c; + } + + public Gee.HashMap to_hash_map(Gee.MapFunc key_func, + owned Gee.HashDataFunc? key_hash_func = null, + owned Gee.EqualDataFunc? key_equal_func = null, + owned Gee.EqualDataFunc? value_equal_func = null) { + return (Gee.HashMap) add_all_to_map(new Gee.HashMap( + key_hash_func, key_equal_func, value_equal_func), key_func); + } +}