openlp-core team mailing list archive

[jerryb <gerald.britton@xxxxxxxxx>]

jerryb has proposed merging lp:~gerald-britton/openlp/connect into lp:openlp.

Requested reviews:
  OpenLP Core (openlp-core)

For more details, see:
https://code.launchpad.net/~gerald-britton/openlp/connect/+merge/60106

Request for Comments Only:
==========================

This patch follows the example set by the "translate" function in core/lib/__init__.py by adding a "connect" function to it.  The purpose of the new function is similar to that of the "translate" function but focussed on signal connections.  

I illustrate the use of the function in core/ui/servicemanager.py.  The net result is shorter expressions (which I find easier to read though others might not agree!).  Additionally, I show the use of functools.partial to curry the "sender" parameter when it is repeated in several lines.

Benefits:

1. Single lookup of QtCore.QObject.connect and QtCore.SIGNAL functions, at function definition time
2. Automatic call to QtCore.SIGNAL for most frequent use cases.
3. Shorter expressions in source code for possible readability improvement.
4. Shorter generated byte code due to pre-resolution of Qt function references.
5. Faster lookup of connect method at execution time since it is in the module namespace.

Note: This is only a request for comments.


-- 
https://code.launchpad.net/~gerald-britton/openlp/connect/+merge/60106
Your team OpenLP Core is requested to review the proposed merge of lp:~gerald-britton/openlp/connect into lp:openlp.

=== modified file 'openlp/core/lib/__init__.py'
--- openlp/core/lib/__init__.py	2011-05-03 17:36:20 +0000
+++ openlp/core/lib/__init__.py	2011-05-05 18:41:32 +0000
@@ -105,6 +105,26 @@
     """
     return translate(context, text, comment, encoding, n)
 
+def connect(sender, signal, method,
+        SIGNAL=QtCore.SIGNAL,
+        connect=QtCore.QObject.connect):
+    """
+    A special shortcut method to wrap around the Qt4 signal connection function.
+    This abstracts the connection procedure so that we can change it if at a
+    later date if necessary, without having to redo the whole of OpenLP.
+
+    ``sender``
+        The object sending the signal.
+
+    ``signal``
+        The signal being sent.  This is a string that will be replaced by the
+        value returned by QtCore.SIGNAL when called with the signal string.
+
+    ``method``
+        The method in the object to receive the signal.
+    """
+    return connect(sender, SIGNAL(signal), method)
+
 def get_text_file_string(text_file):
     """
     Open a file and return its content as unicode string. If the supplied file

=== modified file 'openlp/core/ui/servicemanager.py'
--- openlp/core/ui/servicemanager.py	2011-04-29 08:45:36 +0000
+++ openlp/core/ui/servicemanager.py	2011-05-05 18:41:32 +0000
@@ -33,7 +33,7 @@
 from PyQt4 import QtCore, QtGui
 
 from openlp.core.lib import OpenLPToolbar, ServiceItem, Receiver, build_icon, \
-    ItemCapabilities, SettingsManager, translate
+    ItemCapabilities, SettingsManager, translate, connect
 from openlp.core.lib.theme import ThemeLevel
 from openlp.core.lib.ui import UiStrings, critical_error_message_box, \
     context_menu_action, find_and_set_in_combo_box
@@ -42,6 +42,7 @@
 from openlp.core.utils import AppLocation, delete_file, file_is_unicode, \
     split_filename
 from openlp.core.utils.actions import ActionList, CategoryOrder
+from functools import partial
 
 class ServiceManagerList(QtGui.QTreeWidget):
     """
@@ -150,8 +151,7 @@
         self.serviceManagerList.setExpandsOnDoubleClick(False)
         self.serviceManagerList.setContextMenuPolicy(
             QtCore.Qt.CustomContextMenu)
-        QtCore.QObject.connect(self.serviceManagerList,
-            QtCore.SIGNAL('customContextMenuRequested(QPoint)'),
+        connect(self.serviceManagerList, 'customContextMenuRequested(QPoint)',
             self.contextMenu)
         self.serviceManagerList.setObjectName(u'serviceManagerList')
         # enable drop
@@ -256,35 +256,28 @@
             self.serviceManagerList.makeLive, UiStrings().Service)
         self.layout.addWidget(self.orderToolbar)
         # Connect up our signals and slots
-        QtCore.QObject.connect(self.themeComboBox,
-            QtCore.SIGNAL(u'activated(int)'), self.onThemeComboBoxSelected)
-        QtCore.QObject.connect(self.serviceManagerList,
-            QtCore.SIGNAL(u'doubleClicked(QModelIndex)'), self.onMakeLive)
-        QtCore.QObject.connect(self.serviceManagerList,
-           QtCore.SIGNAL(u'itemCollapsed(QTreeWidgetItem*)'), self.collapsed)
-        QtCore.QObject.connect(self.serviceManagerList,
-           QtCore.SIGNAL(u'itemExpanded(QTreeWidgetItem*)'), self.expanded)
-        QtCore.QObject.connect(Receiver.get_receiver(),
-            QtCore.SIGNAL(u'theme_update_list'), self.updateThemeList)
-        QtCore.QObject.connect(Receiver.get_receiver(),
-            QtCore.SIGNAL(u'servicemanager_preview_live'), self.previewLive)
-        QtCore.QObject.connect(Receiver.get_receiver(),
-            QtCore.SIGNAL(u'servicemanager_next_item'), self.nextItem)
-        QtCore.QObject.connect(Receiver.get_receiver(),
-            QtCore.SIGNAL(u'servicemanager_previous_item'), self.previousItem)
-        QtCore.QObject.connect(Receiver.get_receiver(),
-            QtCore.SIGNAL(u'servicemanager_set_item'), self.onSetItem)
-        QtCore.QObject.connect(Receiver.get_receiver(),
-            QtCore.SIGNAL(u'servicemanager_list_request'), self.listRequest)
-        QtCore.QObject.connect(Receiver.get_receiver(),
-            QtCore.SIGNAL(u'config_updated'), self.configUpdated)
-        QtCore.QObject.connect(Receiver.get_receiver(),
-            QtCore.SIGNAL(u'config_screen_changed'),
+        connect(self.themeComboBox, u'activated(int)',
+            self.onThemeComboBoxSelected)
+        connect(self.serviceManagerList, u'doubleClicked(QModelIndex)',
+            self.onMakeLive)
+        connect(self.serviceManagerList, u'itemCollapsed(QTreeWidgetItem*)',
+            self.collapsed)
+        connect(self.serviceManagerList, u'itemExpanded(QTreeWidgetItem*)',
+            self.expanded)
+
+        # define partial function for following connections
+        connect_receiver = partial(connect, Receiver.get_receiver())
+        connect_receiver(u'theme_update_list', self.updateThemeList)
+        connect_receiver(u'servicemanager_preview_live', self.previewLive)
+        connect_receiver(u'servicemanager_next_item', self.nextItem)
+        connect_receiver(u'servicemanager_previous_item', self.previousItem)
+        connect_receiver(u'servicemanager_set_item', self.onSetItem)
+        connect_receiver(u'servicemanager_list_request', self.listRequest)
+        connect_receiver(u'config_updated', self.configUpdated)
+        connect_receiver(u'config_screen_changed',
             self.regenerateServiceItems)
-        QtCore.QObject.connect(Receiver.get_receiver(),
-            QtCore.SIGNAL(u'theme_update_global'), self.themeChange)
-        QtCore.QObject.connect(Receiver.get_receiver(),
-            QtCore.SIGNAL(u'service_item_update'), self.serviceItemUpdate)
+        connect_receiver(u'theme_update_global', self.themeChange)
+        connect_receiver(u'service_item_update', self.serviceItemUpdate)
         # Last little bits of setting up
         self.service_theme = unicode(QtCore.QSettings().value(
             self.mainwindow.serviceSettingsSection + u'/service theme',