r47062 - review comments

Author: hawkowl
Date: Thu Mar 24 05:28:49 2016
New Revision: 47062

Modified:
   branches/oldstyle-decorator-8244/twisted/python/_oldstyle.py
   branches/oldstyle-decorator-8244/twisted/python/test/test_util.py
   branches/oldstyle-decorator-8244/twisted/python/util.py
   branches/oldstyle-decorator-8244/twisted/test/test_nooldstyle.py

Log:
review comments

Modified: branches/oldstyle-decorator-8244/twisted/python/_oldstyle.py
==============================================================================
--- branches/oldstyle-decorator-8244/twisted/python/_oldstyle.py	(original)
+++ branches/oldstyle-decorator-8244/twisted/python/_oldstyle.py	Thu Mar 24 05:28:49 2016
@@ -12,6 +12,18 @@
 import types
 
 from twisted.python.compat import _PY3
+from twisted.python.util import _replaceIf
+
+
+def passthru(arg):
+    """
+    Return C{arg}. Do nothing.
+
+    @param arg: The arg to return.
+    @return: C{arg}
+    """
+    return arg
+
 
 
 def _ensureOldClass(cls):
@@ -20,7 +32,7 @@
 
     @param cls: The class to check.
 
-    @return: C{None} if it is an old-style class.
+    @return: The class, if it is an old-style class.
     @raises: L{ValueError} if it is a new-style class.
     """
     if not type(cls) is types.ClassType:
@@ -32,45 +44,21 @@
              "decorate old-style classes.").format(
                  cls=fullyQualifiedName(cls)))
 
+    return cls
 
-if _PY3:
 
-    def _oldStyle(cls):
-        """
-        No such thing as an old style class on Python 3.
-
-        @param cls: The class to wrap (or in this case, not wrap).
-        @return: C{cls}, unchanged
-        """
-        return cls
-
-elif int(os.environ.get('TWISTED_NEWSTYLE', 0)) == 0:
-
-    def _oldStyle(cls):
-        """
-        We don't want to override anything, but throw an exception if a
-        new-style class is decorated.
-
-        @param cls: The class to wrap (or in this case, not wrap).
-        @type cls: L{types.ClassType}
-
-        @return: C{cls}, unchanged
-        @raises: L{ValueError} if C{cls} is a new-style class.
-        """
-        _ensureOldClass(cls)
-        return cls
-
-else:
-
-    def _oldStyle(cls):
-        """
-        A decorator which converts old-style classes to new-style classes.
-
-        @param cls: An old-style class to convert to new-style.
-        @type cls: L{types.ClassType}
-        @return: A new-style subclass of C{cls}.
-        """
-        _ensureOldClass(cls)
 
-        OverwrittenClass = type(cls.__name__, (object,), cls.__dict__)
-        return OverwrittenClass
+@_replaceIf(_PY3, passthru)
+@_replaceIf(int(os.environ.get('TWISTED_NEWSTYLE', 0)) == 0, _ensureOldClass)
+def _oldStyle(cls):
+    """
+    A decorator which conditionally converts old-style classes to new-style
+    classes. If it is Python 3, or if "TWISTED_NEWSTYLE" is not set or has a 0
+    value in the environment, this decorator is a no-op.
+
+    @param cls: An old-style class to convert to new-style.
+    @type cls: L{types.ClassType}
+    @return: A new-style version of C{cls}.
+    """
+    _ensureOldClass(cls)
+    return type(cls.__name__, (object,), cls.__dict__)

Modified: branches/oldstyle-decorator-8244/twisted/python/test/test_util.py
==============================================================================
--- branches/oldstyle-decorator-8244/twisted/python/test/test_util.py	(original)
+++ branches/oldstyle-decorator-8244/twisted/python/test/test_util.py	Thu Mar 24 05:28:49 2016
@@ -1117,3 +1117,61 @@
         items = []
         util.padTo(4, items)
         self.assertEqual([], items)
+
+
+class ReplaceIfTests(unittest.TestCase):
+
+    def test_replacesIfTrue(self):
+        """
+        L{util._replaceIf} swaps out the body of a function if the conditional
+        is C{True}.
+        """
+        @util._replaceIf(True, lambda: "hi")
+        def test():
+            return "bye"
+
+        self.assertEqual(test(), "hi")
+        self.assertEqual(test.__name__, "test")
+        self.assertEqual(test.__module__, "twisted.python.test.test_util")
+
+
+    def test_keepsIfFalse(self):
+        """
+        L{util._replaceIf} keeps the original body of the function if the
+        conditional is C{False}.
+        """
+        @util._replaceIf(False, lambda: "hi")
+        def test():
+            return "bye"
+
+        self.assertEqual(test(), "bye")
+
+
+    def test_multipleReplace(self):
+        """
+        In the case that multiple conditions are true, the first one
+        (to the reader) is chosen by L{util._replaceIf}
+        """
+        @util._replaceIf(True, lambda: "hi")
+        @util._replaceIf(False, lambda: "bar")
+        @util._replaceIf(True, lambda: "baz")
+        def test():
+            return "bye"
+
+        self.assertEqual(test(), "hi")
+
+
+    def test_boolsOnly(self):
+        """
+        L{util._replaceIf}'s condition argument only accepts bools.
+        """
+        with self.assertRaises(ValueError) as e:
+
+            @util._replaceIf("hi", "there")
+            def test():
+                """
+                Some test function.
+                """
+
+        self.assertEqual(e.exception.args[0],
+                         "condition argument to _replaceIf requires a bool.")

Modified: branches/oldstyle-decorator-8244/twisted/python/util.py
==============================================================================
--- branches/oldstyle-decorator-8244/twisted/python/util.py	(original)
+++ branches/oldstyle-decorator-8244/twisted/python/util.py	Thu Mar 24 05:28:49 2016
@@ -14,6 +14,8 @@
 except ImportError:
     setgroups = getgroups = None
 
+from functools import wraps
+
 from twisted.python.compat import _PY3, unicode
 from twisted.python.versions import Version
 from twisted.python.deprecate import deprecatedModuleAttribute
@@ -905,6 +907,35 @@
 
 
 
+def _replaceIf(condition, alternative):
+    """
+    If C{condition}, replace this function with C{alternative}.
+
+    @param condition: A L{bool} which says whether this should be replaced.
+    @param alternative: An alternative function that will be swapped in instead
+    of the original, if C{condition} is truthy.
+    @return: A decorator.
+    """
+    def decorator(func):
+
+        if condition is True:
+            call = alternative
+        elif condition is False:
+            call = func
+        else:
+            raise ValueError(
+                "condition argument to _replaceIf requires a bool.")
+
+        @wraps(func)
+        def wrapped(*args, **kwargs):
+            return call(*args, **kwargs)
+
+        return wrapped
+
+    return decorator
+
+
+
 __all__ = [
     "uniquify", "padTo", "getPluginDirs", "addPluginDir", "sibpath",
     "getPassword", "println", "makeStatBar", "OrderedDict",

Modified: branches/oldstyle-decorator-8244/twisted/test/test_nooldstyle.py
==============================================================================
--- branches/oldstyle-decorator-8244/twisted/test/test_nooldstyle.py	(original)
+++ branches/oldstyle-decorator-8244/twisted/test/test_nooldstyle.py	Thu Mar 24 05:28:49 2016
@@ -33,6 +33,11 @@
     bar = "baz"
 
     def func(self):
+        """
+        A function on a old style class.
+
+        @return: "hi", for testing.
+        """
         return "hi"