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



commit ea9f4b06393425e7496a3494e084520da3de7618
Author: Jonathon Jongsma <jonathon jongsma collabora co uk>
Date:   Fri Feb 13 11:35:42 2009 -0600

    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.

 libempathy-gtk/empathy-contact-list-store.c |   82 ++++++++++++--------------
 1 files changed, 38 insertions(+), 44 deletions(-)
---
diff --git a/libempathy-gtk/empathy-contact-list-store.c b/libempathy-gtk/empathy-contact-list-store.c
index 417250f..c516dbf 100644
--- a/libempathy-gtk/empathy-contact-list-store.c
+++ b/libempathy-gtk/empathy-contact-list-store.c
@@ -803,6 +803,38 @@ contact_list_store_inibit_active_cb (EmpathyContactListStore *store)
 }
 
 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,
 				       EmpathyContact          *actor,
@@ -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



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]