[Bug 33525] Helper class(es) for observing calls

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Wed Apr 27 15:20:08 CEST 2011


https://bugs.freedesktop.org/show_bug.cgi?id=33525

--- Comment #9 from Olli Salli <ollisal at gmail.com> 2011-04-27 06:20:05 PDT ---
Starting to look even better, though we definitely still need the tests. Some
things though:

+    foreach (const ChannelPtr &channel, newChannels) {
+        if (filterChannel(channelsAccount, channel)) {
+            match.insert(channel);
+        }
+    }
+
+    if (match.isEmpty() || channels.contains(match)) {
+        return;
+    }
+

Do if (!channels.contains(channel) && filterChannel(... in the loop already and
only check for match being empty afterwards to not emit redundant additions in
the signal? (When you get newChannels in which some of them are new and some
existed already). That'd be also slightly faster than checking the channels and
match sets for being a subset (set.contains(set)).

+void SimpleObserver::Private::removeChannel(const AccountPtr &channelAccount,
+        const ChannelPtr &channel,

Similarly, skip the filtering and possible emitting if channels doesn't contain
the channel anymore in the first place (i.e. redundant channel invalidation).
This wouldn't likely happen in practice, though, unlike the first one, but
would be good for consistency / clarity.

> Properly install ChannelCLassFeatures instead of ChannelFeatureSpec.

As it wasn't possible to move the typedef to types.h, you need to install
channel-class-features.h too.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.



More information about the telepathy-bugs mailing list