Eliminate if-tests for Types in Builder.execute().

2 files changed, 48 insertions, 16 deletions

diff --git a/src/scons/Builder.py b/src/scons/Builder.py
index 76c5512..ba489ec 100644
--- a/src/scons/Builder.py
+++ b/src/scons/Builder.py
@@ -9,7 +9,7 @@ __revision__ = "Builder.py __REVISION__ __DATE__ __DEVELOPER__"
 
 
 import os
-from types import *
+import types
 from scons.Node.FS import Dir, File, lookup
 
 
@@ -25,7 +25,7 @@ class Builder:
 			output_suffix = None,
 			node_class = File):
 	self.name = name
-	self.action = action
+	self.action = Action(action)
 	self.insuffix = input_suffix
 	self.outsuffix = output_suffix
 	self.node_class = node_class
@@ -46,13 +46,45 @@ class Builder:
     def execute(self, **kw):
 	"""Execute a builder's action to create an output object.
 	"""
-	# XXX THIS SHOULD BE DONE BY TURNING Builder INTO A FACTORY
-	# FOR SUBCLASSES FOR StringType AND FunctionType
-	t = type(self.action)
-	if t == StringType:
-	    cmd = self.action % kw
-	    print cmd
-	    os.system(cmd)
-	elif t == FunctionType:
-	    # XXX WHAT SHOULD WE PRINT HERE
-	    self.action(kw)
+	apply(self.action.execute, (), kw)
+
+
+
+def Action(act):
+    """A factory for action objects."""
+    if type(act) == types.FunctionType:
+	return FunctionAction(act)
+    elif type(act) == types.StringType:
+	return CommandAction(act)
+    else:
+	return None
+
+class ActionBase:
+    """Base class for actions that create output objects.
+    
+    We currently expect Actions will only be accessible through
+    Builder objects, so they don't yet merit their own module."""
+    def __cmp__(self, other):
+	return cmp(self.__dict__, other.__dict__)
+
+    def show(self, string):
+	print string
+
+class CommandAction(ActionBase):
+    """Class for command-execution actions."""
+    def __init__(self, string):
+	self.command = string
+
+    def execute(self, **kw):
+	cmd = self.command % kw
+	self.show(cmd)
+	os.system(cmd)
+
+class FunctionAction(ActionBase):
+    """Class for Python function actions."""
+    def __init__(self, function):
+	self.function = function
+
+    def execute(self, **kw):
+	# XXX:  WHAT SHOULD WE PRINT HERE?
+	self.function(kw)
diff --git a/src/scons/BuilderTests.py b/src/scons/BuilderTests.py
index 1effa89..822b64c 100644
--- a/src/scons/BuilderTests.py
+++ b/src/scons/BuilderTests.py
@@ -3,8 +3,8 @@ __revision__ = "BuilderTests.py __REVISION__ __DATE__ __DEVELOPER__"
 import sys
 import unittest
 
-from scons.Builder import Builder
-from TestCmd import TestCmd
+import TestCmd
+from scons.Builder import Builder, CommandAction, FunctionAction
 
 
 # Initial setup of the common environment for all tests,
@@ -14,7 +14,7 @@ from TestCmd import TestCmd
 # We don't do this as a setUp() method because it's
 # unnecessary to create a separate directory and script
 # for each test, they can just use the one.
-test = TestCmd(workdir = '')
+test = TestCmd.TestCmd(workdir = '')
 
 test.write('act.py', """import os, string, sys
 f = open(sys.argv[1], 'w')
@@ -35,7 +35,7 @@ class BuilderTestCase(unittest.TestCase):
 	Verify that we can retrieve the supplied action attribute.
 	"""
 	builder = Builder(action = "foo")
-	assert builder.action == "foo"
+	assert builder.action.command == "foo"
 
     def test_cmp(self):
 	"""Test simple comparisons of Builder objects