launchpad-reviewers team mailing list archive

[Данило Шеган <danilo@xxxxxxxxxxxxx>]

Данило Шеган has proposed merging lp:~danilo/pygettextpo/simplify into lp:pygettextpo.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~danilo/pygettextpo/simplify/+merge/86870

Simplify the C code behind simple set_msg* methods to facilitate code reuse.

Since I'd like to extend it to support all the other current features of libgettextpo, and a bunch of other methods would need the same code for executing a bunch of actions (set_msgctxt, set_extracted_comments...).

Alternative approach for this would have been to define a macro, which might be faster (no passing around of function pointers), but it would have resulted in larger binaries and would have been uglier (\ continuation lines and such).

'make check' confirms no tests are broken (all the modified methods are tested in a basic way, but that should be sufficient for the type of changes here).
-- 
https://code.launchpad.net/~danilo/pygettextpo/simplify/+merge/86870
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~danilo/pygettextpo/simplify into lp:pygettextpo.

=== modified file 'gettextpo.c'
--- gettextpo.c	2009-06-02 15:31:25 +0000
+++ gettextpo.c	2011-12-25 19:30:30 +0000
@@ -533,35 +533,46 @@
     return res;
 }
 
+typedef void (po_value_setter_t)(po_message_t, const char *);
+
+static PyObject *
+helper_message_set_field(PyPoMessage *self, PyObject *args, const char *field,
+                         po_value_setter_t * setter)
+{
+    const char *value;
+    PyObject *object;
+    PyObject *string;
+
+    if (!PyArg_ParseTuple(args, field, &object))
+        return NULL;
+
+    if (object == Py_None) {
+        (*setter)(self->msg, NULL);
+    } else {
+        string = get_pystring_from_pyobject(object);
+
+        if (string == NULL)
+            /* Got an exception */
+            return NULL;
+        else {
+            value = PyString_AsString(string);
+            (*setter)(self->msg, value);
+            Py_DECREF(string);
+        }
+    }
+
+    Py_RETURN_NONE;
+}
+
+
 PyDoc_STRVAR(doc_pypo_message_set_msgid,
 "M.set_msgid(msgid) -> None.  Set the msgid for this PoMessage");
 
 static PyObject *
 pypo_message_set_msgid(PyPoMessage *self, PyObject *args)
 {
-    const char *msgid;
-    PyObject *object;
-    PyObject *string;
-
-    if (!PyArg_ParseTuple(args, "O:set_msgid", &object))
-        return NULL;
-
-    if (object == Py_None) {
-        po_message_set_msgid(self->msg, NULL);
-    } else {
-        string = get_pystring_from_pyobject(object);
-
-        if (string == NULL)
-            /* Got an exception */
-            return NULL;
-        else {
-            msgid = PyString_AsString(string);
-            po_message_set_msgid(self->msg, msgid);
-            Py_DECREF(string);
-        }
-    }
-
-    Py_RETURN_NONE;
+    return helper_message_set_field(self, args, "O:set_msgid",
+                                    &po_message_set_msgid);
 }
 
 PyDoc_STRVAR(doc_pypo_message_set_msgid_plural,
@@ -571,29 +582,8 @@
 static PyObject *
 pypo_message_set_msgid_plural(PyPoMessage *self, PyObject *args)
 {
-    const char *msgid;
-    PyObject *object;
-    PyObject *string;
-
-    if (!PyArg_ParseTuple(args, "O:set_msgid_plural", &object))
-        return NULL;
-
-    if (object == Py_None) {
-        po_message_set_msgid_plural(self->msg, NULL);
-    } else {
-        string = get_pystring_from_pyobject(object);
-
-        if (string == NULL)
-            /* Got an exception */
-            return NULL;
-        else {
-            msgid = PyString_AsString(string);
-            po_message_set_msgid_plural(self->msg, msgid);
-            Py_DECREF(string);
-        }
-    }
-
-    Py_RETURN_NONE;
+    return helper_message_set_field(self, args, "O:set_msgid_plural",
+                                    &po_message_set_msgid_plural);
 }
 
 PyDoc_STRVAR(doc_pypo_message_set_msgstr,
@@ -602,29 +592,8 @@
 static PyObject *
 pypo_message_set_msgstr(PyPoMessage *self, PyObject *args)
 {
-    const char *msgstr;
-    PyObject *object;
-    PyObject *string;
-
-    if (!PyArg_ParseTuple(args, "O:set_msgstr", &object))
-        return NULL;
-
-    if (object == Py_None) {
-        po_message_set_msgstr(self->msg, NULL);
-    } else {
-        string = get_pystring_from_pyobject(object);
-
-        if (string == NULL)
-            /* Got an exception */
-            return NULL;
-        else {
-            msgstr = PyString_AsString(string);
-            po_message_set_msgstr(self->msg, msgstr);
-            Py_DECREF(string);
-        }
-    }
-
-    Py_RETURN_NONE;
+    return helper_message_set_field(self, args, "O:set_msgstr",
+                                    &po_message_set_msgstr);
 }
 
 PyDoc_STRVAR(doc_pypo_message_set_msgstr_plural,
@@ -675,29 +644,8 @@
 static PyObject *
 pypo_message_set_comments(PyPoMessage *self, PyObject *args)
 {
-    const char *comments;
-    PyObject *object;
-    PyObject *string;
-
-    if (!PyArg_ParseTuple(args, "O:set_comments", &object))
-        return NULL;
-
-    if (object == Py_None) {
-        po_message_set_comments(self->msg, NULL);
-    } else {
-        string = get_pystring_from_pyobject(object);
-
-        if (string == NULL)
-            /* Got an exception */
-            return NULL;
-        else {
-            comments = PyString_AsString(string);
-            po_message_set_comments(self->msg, comments);
-            Py_DECREF(string);
-        }
-    }
-
-    Py_RETURN_NONE;
+    return helper_message_set_field(self, args, "O:set_comments",
+                                    &po_message_set_comments);
 }
 
 PyDoc_STRVAR(doc_pypo_message_set_format,