Fixed some of Gillaume's review comments from Bug #571642

The following comments should be fixed in this commit:
- tp_group_update_members: add "old" and "new" in comments so we known which
  contact we are talking about.
- Always use TpHandle instead of guint when storing handles in variables.
- contact_list_store_member_renamed_cb: would be good to share the common code
  with contact_list_store_members_changed_cb in a common function

One comment about the last one: I split out common code into two new functions:
contact_list_store_add_contact_and_connect(), and the matching
contact_list_store_remove_contact_and_disconnect().  I wondered whether the
signal connection/disconnection should just be added to the _add_contact() and
_remove_contact() functions directly, but they do seem to be used in other
places without connecting to signals, so i thought it would be safer to simply
add some wrapper functions.

diff --git a/libempathy-gtk/empathy-contact-list-store.c b/libempathy-gtk/empathy-contact-list-store.c
index 417250fcd7fa879cfa9f49110fdcb044858f7b05..c516dbf7cff8f87c4ff432072b31f6863bd2e727 100644 (file)
--- a/libempathy-gtk/empathy-contact-list-store.c
+++ b/libempathy-gtk/empathy-contact-list-store.c
@@ -802,6 +802,38 @@ contact_list_store_inibit_active_cb (EmpathyContactListStore *store)
        return FALSE;
 }
 
+static void
+contact_list_store_add_contact_and_connect (EmpathyContactListStore *store, EmpathyContact *contact)
+{
+       g_signal_connect (contact, "notify::presence",
+                         G_CALLBACK (contact_list_store_contact_updated_cb),
+                         store);
+       g_signal_connect (contact, "notify::presence-message",
+                         G_CALLBACK (contact_list_store_contact_updated_cb),
+                         store);
+       g_signal_connect (contact, "notify::name",
+                         G_CALLBACK (contact_list_store_contact_updated_cb),
+                         store);
+       g_signal_connect (contact, "notify::avatar",
+                         G_CALLBACK (contact_list_store_contact_updated_cb),
+                         store);
+       g_signal_connect (contact, "notify::capabilities",
+                         G_CALLBACK (contact_list_store_contact_updated_cb),
+                         store);
+
+       contact_list_store_add_contact (store, contact);
+}
+
+static void
+contact_list_store_remove_contact_and_disconnect (EmpathyContactListStore *store, EmpathyContact *contact)
+{
+       g_signal_handlers_disconnect_by_func (contact,
+                                             G_CALLBACK (contact_list_store_contact_updated_cb),
+                                             store);
+
+       contact_list_store_remove_contact (store, contact);
+}
+
 static void
 contact_list_store_members_changed_cb (EmpathyContactList      *list_iface,
                                       EmpathyContact          *contact,
@@ -821,29 +853,9 @@ contact_list_store_members_changed_cb (EmpathyContactList      *list_iface,
                is_member ? "added" : "removed");
 
        if (is_member) {
-               g_signal_connect (contact, "notify::presence",
-                                 G_CALLBACK (contact_list_store_contact_updated_cb),
-                                 store);
-               g_signal_connect (contact, "notify::presence-message",
-                                 G_CALLBACK (contact_list_store_contact_updated_cb),
-                                 store);
-               g_signal_connect (contact, "notify::name",
-                                 G_CALLBACK (contact_list_store_contact_updated_cb),
-                                 store);
-               g_signal_connect (contact, "notify::avatar",
-                                 G_CALLBACK (contact_list_store_contact_updated_cb),
-                                 store);
-               g_signal_connect (contact, "notify::capabilities",
-                                 G_CALLBACK (contact_list_store_contact_updated_cb),
-                                 store);
-
-               contact_list_store_add_contact (store, contact);
+               contact_list_store_add_contact_and_connect (store, contact);
        } else {
-               g_signal_handlers_disconnect_by_func (contact,
-                                                     G_CALLBACK (contact_list_store_contact_updated_cb),
-                                                     store);
-
-               contact_list_store_remove_contact (store, contact);
+               contact_list_store_remove_contact_and_disconnect (store, contact);
        }
 }
 
@@ -865,29 +877,11 @@ contact_list_store_member_renamed_cb (EmpathyContactList      *list_iface,
                empathy_contact_get_id (new_contact),
                empathy_contact_get_handle (new_contact));
 
-       /* connect to signals of new contact */
-       g_signal_connect (new_contact, "notify::presence",
-                         G_CALLBACK (contact_list_store_contact_updated_cb),
-                         store);
-       g_signal_connect (new_contact, "notify::presence-message",
-                         G_CALLBACK (contact_list_store_contact_updated_cb),
-                         store);
-       g_signal_connect (new_contact, "notify::name",
-                         G_CALLBACK (contact_list_store_contact_updated_cb),
-                         store);
-       g_signal_connect (new_contact, "notify::avatar",
-                         G_CALLBACK (contact_list_store_contact_updated_cb),
-                         store);
-       g_signal_connect (new_contact, "notify::capabilities",
-                         G_CALLBACK (contact_list_store_contact_updated_cb),
-                         store);
-       contact_list_store_add_contact (store, new_contact);
+       /* add the new contact */
+       contact_list_store_add_contact_and_connect (store, new_contact);
 
-       /* disconnect from old contact */
-       g_signal_handlers_disconnect_by_func (old_contact,
-                                             G_CALLBACK (contact_list_store_contact_updated_cb),
-                                             store);
-       contact_list_store_remove_contact (store, old_contact);
+       /* remove old contact */
+       contact_list_store_remove_contact_and_disconnect (store, old_contact);
 }
 
 static void