Page 1 of 2

Building a FrontController Test

Posted: Mon Oct 03, 2005 5:19 pm
by Maugrim_The_Reaper
To start I'm a complete newb when it comes to TDD, so what follows just may be a stupid question. You've been warned ;)

In building a test for an FC I seem to be having trouble in ascertaining whether one method has made calls to two other class methods (all in same FC class).

To start the FC has a processRequest() method. This in turn makes calls to parseAction() and selectCommandClass(). Now how can I write a test case (on processRequest()) to ascertain that a call was made to the other two methods?

Test to date (I know about the desperate call to the mock class :)):

Code: Select all

require_once(APPBASE . 'common.inc.php'); // a common file linking to backend lib - required

require_once(APPBASE . 'System/FrontController/FrontController.php');
Mock::generate('FrontController');


class TestOf_FrontController extends UnitTestCase {

	var $_requestarray;

	function TestOf_FrontController() {
		$this->UnitTestCase('System => FrontController Test');
	}

	function SetUp() {
		$_requestarray = $_REQUEST;
		$_REQUEST['action'] = 'system.signup.process';
	}

	function tearDown() {
		$_REQUEST = $_requestarray;
	}

	function testContructor() {
		$front =& new FrontController($this);
		$this->assertEqual($front->privatePage,true); //pass
	}

	function testExecute() {
		// ... intercepting filter, etc. down to processRequest()
	}

	function testProcessRequest() {
		// expected: FC should call parseAction() and selectCommand() in processRequest()
			// how to accomplish?
		$front =& new MockFrontController($this);
		$front->expectOnce('parseAction', array());
		$front->expectOnce('selectCommandClass', array());
		$front->processRequest(); //the desperate "let's give it a long shot" error 
		$front->tally();
	}

	function testSelectCommandClass() {
		// ...
	}

	function testParseAction() {
		$this->assertWantedPattern('/^[-A-Z\.]*$/i', $_REQUEST['action']);
		$front =& new FrontController($this);
		$front->parseAction();
		$this->assertEqual($front->module,'System'); //pass
		$this->assertEqual($front->command,'SignupProcessCommand'); //pass
	}
}
I'm not arguing the pros and cons of the FC here (this is a simple 100 line FC or FC like class) - just the test method, or design changes that might be getting suggested by above difficulty (if).

Posted: Mon Oct 03, 2005 5:49 pm
by McGruff
Close - but you can't mock the primary class (ie the class under test). Mocking an object knocks out all its methods: it's entirely replaced with the "fake" mock object.

You could partially mock in order to knock out selected methods. As you can see on Marcus' site, you can use partial mocks to substitute a (second) mock for an object which the primary class will create in a factory method. This is the TDD bit: the object doesn't have to be written while you test the primary class. The process of testing the primary class will allow you to discover an interface for the mocked object and describe the behaviour which it should exhibit when you do come to implement it.

What's the simplest interface you can think of for a FrontController?

Posted: Mon Oct 03, 2005 5:52 pm
by Weirdan
From what I read of unit testing you should test the public interfaces only. parseAction and selectCommandClass look like private methods...

You might try partial mocks though

Posted: Mon Oct 03, 2005 5:58 pm
by McGruff
That's what I was getting at.

TDD is an interaction style though so you do peek under the bonnet a little bit. You have to in order to figure out interfaces & behaviours for the next layer of objects.

Posted: Tue Oct 04, 2005 3:21 am
by Maugrim_The_Reaper
I see some additional feet of learning curve dead ahead...;)

Ok, so partial mocks will get me past my issue...fine. I'm getting somewhere at least. To another question why should TDD apply to public methods more than privates - surely private methods can influence the public methods quite a bit. I mean in many cases, private methods are code from publics which have just been separated out to make the class logic more clear cut.

Why would I not test private methods? :? Or rather WHEN would I not test them...

(Have a feeling we're heading towards the scenario where my tests would (if done from scratch) have been testing those privates when their source was included in the original public method before being moved to a private method)

Posted: Tue Oct 04, 2005 6:03 am
by Maugrim_The_Reaper
I seem to be stumbling into something of a debate - after a little googling there's a great deal of material on if or when a private method should be tested separately from the public methods. It should be valid to run the tests within the public method (since it call the private methods), however me being a great believer in discrete utility units that doesn't really make a huge amount of sense. Still reading up on it...

For a piddling little class like above...maybe. But what when those privates grow to a dozen or more feeding into maybe one or two public classes. Especially when all private methods are very specific to the class (to the point of not being re-usable in any other class). So jumping to a new class makes little sense - it'd only add more complexity, and all to pass a test. So the logical approach is test the private methods independent of the public methods that use them...

Have to say less than a day into TDD and its proving a surprise. It really does force you to really see where the design starts falling down... It's already pointed out that I should have a separate class for mapping request to command. It also threw out some more changes...

I see the solution after more work as:

Code: Select all

require_once(APPBASE . 'common.inc.php'); // a common file linking to backend lib - required

require_once(APPBASE . 'System/FrontController/FrontController.php');
require_once(APPBASE . 'System/RequestMapper/RequestMapper.php');
Mock::generatePartial('FrontController', 'FrontController_Partial', array('createCommand', 'getPrivate'));
Mock::generate('RequestMapper');

require_once(APPBASE . 'System/CommandBase/CommandBase.php');
require_once(APPBASE . 'Modules/Index/DefaultCommand.php');
Mock::generate('DefaultCommand');


class TestOf_FrontController extends UnitTestCase {

    function TestOf_FrontController() {
        $this->UnitTestCase('System => FrontController Test');
    }

    function testContructor() {
        $front =& new FrontController( new MockRequestMapper() );
        $this->assertEqual($front->privatePage,true); //pass
    }

    function testExecute() {
        // ... intercepting filter, etc. down to processRequest()
    }

    function testProcessRequest() {
		$reqMap =& new MockRequestMapper($this);
		$reqMap->setReturnValue('getCommandPath', '/modulepath/');
		$reqMap->setReturnValue('getCommand', 'SomeCommand');
        $front =& new FrontController_Partial();
		$command =& new MockDefaultCommand();
		$front->setReturnReference('createCommand', $command);
		$front->setReturnValue('getPrivate', true);
        $reqMap->expectOnce('getCommandPath', array());
		$reqMap->expectOnce('getCommand', array());
		$front->expectOnce('createCommand', array());
		$front->expectOnce('getPrivate', array());
		$command->expectOnce('execute', array());
		$command->expectOnce('render', array());
		$command->setReturnValue('getPrivate', true);
		$command->expectOnce('getPrivate', array());
        $front->processRequest($reqMap);
		$this->assertEqual($front->commandPath, '/modulepath/');
		$this->assertEqual($front->command, 'SomeCommand');
        $reqMap->tally();
		$front->tally();
		$command->tally();
    }
}
1/1 test cases complete: 17 passes, 0 fails and 0 exceptions.
To pass some private properties were called from "get" functions, command instantiation was shifted to a factory (probably should be a new class...however), request parsing was shifted to a request mapper (or a simple version of one), all those new changes were amenable to mock objects being used, and now the whole test passes...neato.

Posted: Tue Oct 04, 2005 6:29 am
by Maugrim_The_Reaper
Well, that saw a lot of climbing up the LC. I'll post the implementation code. I note that processRequest this time is private...getting complex...;) The execute method now contains a filter chain (InterceptingFilter Pattern) with the FC's processRequest being the last "filter" called. I don't believe this is "taking away" from an FC - the FC is still handling the request centrally. So in adding an IF just how do we adapt our testing...

Code: Select all

// copyright (c) 2005 Padraic Brady
// released under the GNU General Public License

// FC implementation for my TDD try out

/*
Issues - getting $requestMapper to processRequest() when part of an InterceptingFilter
            - Testing the IF
            - Testing processRequest when a private method called from IF
*/

require_once(APPROOT . 'System/InterceptingFilter/InterceptingFilter.php');

class FrontController extends InterceptingFilter {
	
	var $module;
	var $command;
	var $commandPath;
	var $privatePage;

	function FrontController($private=true) {
		$this->privatePage = $private;
	}

	function execute() {
		// setup our Intercepting Filter chain
		require_once(APPROOT . 'System/InterceptingFilter/Filters/Filter_InputFilter.php');
		if($this->privatePage)
		{
			// a private page requires authentication (assume private unless explicitly set as public)
			// also requires a session expiry check
			require_once(APPROOT . 'System/InterceptingFilter/Filters/Filter_SessionExpiry.php');
			require_once(APPROOT . 'System/InterceptingFilter/Filters/Filter_UserValidation.php');
			$filterChain = new Filter_InputFilter( new Filter_SessionExpiry( new Filter_UserValidation( $this ) ) );
		}
		else 
		{
			// a public page can be viewed without authentication
			$filterChain = new Filter_InputFilter( $this );
		}
		$filterChain->processRequest();
		// exit entire process
		exit();
	}


	/////        $requestMapper should be moved - this is now part of an IF
	function processRequest(&$requestMapper) {
		if(!is_object($requestMapper)) 
		{
			trigger_error('non-Object [$requestMapper] passed to FrontController::processRequest', E_USER_ERROR);
		}
		$this->command = $requestMapper->getCommand();
		$this->module = $requestMapper->getModule();
		$this->commandPath = $requestMapper->getCommandPath();
		$command =& $this->createCommand();
		$c_private = $command->getPrivate();
		if(!isset($c_private) || !is_bool($c_private)) 
		{
			// every command class must define a privateCommand value to set whether Command requires
			// authentication or not!
			trigger_error('The ' . $this->command . ' class has not defined whether this Command Object is private (requires authentication) or public (no authentication) - a ' . $this->command . '::privateCommand property must be set to a boolean value (TRUE or FALSE)', E_USER_ERROR);
		}
		if($this->getPrivate() !== $c_private) 
		{
			// The private flag of both FC and Command must match
			trigger_error('privatePage flag of FrontController does not match privateCommand flag of ' . $this->command . ' object - these must match for a Command to proceed', E_USER_ERROR);
		}
		$command->execute();
		$command->render(); // move tpl parsing to views
	}

	function &createCommand() {
		require_once(APPROOT . 'System/CommandBase/CommandBase.php'); // commands extend this abstract class (this is all PHP4 compliant at mo)
		if(file_exists($this->commandPath . $this->command . '.php'))
		{
			require_once($this->commandPath . $this->command . '.php');
		}
		else 
		{
			// the action's Module directory or Command Object reference is incorrect
			header("HTTP/1.0 404 Not Found"); // not working on current Apache install hmm...
			die('The page you were looking for could not be found.<br /><br />Please return to <a href="'.URLROOT.'">'.URLROOT.'</a>');
		}
		$command =& new $this->command();
		return $command;
	}

	function getPrivate() {
		return $this->privatePage;
	}

}

Posted: Tue Oct 04, 2005 2:17 pm
by McGruff
Maugrim_The_Reaper wrote:there's a great deal of material on if or when a private method should be tested separately from the public methods.
Each assertion creates a constraint which a prospective solution must satisfy. The test case as a whole creates a full spec for possible implementations (test-infected programmers often see the tests as being more important than the actual code). Tests should be implementation-agnostic. You really don't care how the class is implemented so long as everything runs green. In other words you're only testing the public interfaces. This is a state-based test ie you pull a lever and check to see what value is returned, or if some other entity has changed. If you refactor, perhaps discarding some private methods or adding new ones, it won't affect the test case.

At least that's one approach. Test-driven design is fundamentally different. Here, you're not testing so much as exploring a design (conveniently, some tests will be left behind in the wake). Some classes just have a few simple methods and don't call on other objects. At other points in an OOP design, you'll have groups of interacting objects co-operating to get the job done. A FrontController obviously will interact with a variety of objects but we don't, at present know which. We'll just invent them as we go along using an interaction based style of testing - TDD. Using mocks, we can define interfaces and (by setting expectations) describe the kind of behaviour we want them to exhibit. This creates a blueprint when we come to implement the secondary objects - which in turn may have secondary objects of their own and so on and so on. The design ripples out from the point at which you started.

The big difference compared to state-based testing is that we have to make assertions about the internal workings of the primary class - but only at the points where secondary objects are called. This is what allows you to use testing as a design tool.

Fowler has a good article comparing the two testing styles: Mocks aren't Stubs. You don't have to use test driven design and interaction tests, but it is a nice way to work.

You'll always want at least some state based tests - integration tests - to back up the (interaction-based) unit tests.
Maugrim_The_Reaper wrote:Have to say less than a day into TDD and its proving a surprise. It really does force you to really see where the design starts falling down... It's already pointed out that I should have a separate class for mapping request to command. It also threw out some more changes...
Great :) Good classes tend to be lean and focussed, with just one job to do. If not they can become hard to test. Testing profoundly affects your whole approach to programming and, as you say, very often helps you to write better designed code. I think the key is that it encourages you to think more about interfaces and behaviours rather than getting bogged down in implementations.

Posted: Tue Oct 04, 2005 2:34 pm
by McGruff
Maugrim_The_Reaper wrote:I see the solution after more work as:
Don't forget to explictly call the constructor of a partial mock. It won't be called otherwise and the class may not be instantiated correctly. You also have to call the constructor after setting up the partial mock's expectations. it could be that the constructor calls a method which you knocked out with a partial mock, or does something with a mock object you passed to the constructor.

Some minor points. if you're using v1.0 of simple test, you need to pass $this to mock constructors. The latest alpha release simplifies things. You don't need to do that, or make tally() calls.

A very minor point of style is to group the mock set up together, per object, if you agree that's slightly easier to read. You don't also have to group by method, although that's the way I like to do it.

Code: Select all

function testProcessRequest() {
        $command =& new MockDefaultCommand($this); #!! if not using simpletest_1.0.1alpha2
        $command->expectOnce('execute', array());
        $command->expectOnce('render', array());
        $command->expectOnce('getPrivate', array());
        $command->setReturnValue('getPrivate', true);

        $reqMap =& new MockRequestMapper($this);
        $reqMap->expectOnce('getCommandPath', array());
        $reqMap->setReturnValue('getCommandPath', '/modulepath/');
        $reqMap->expectOnce('getCommand', array());
        $reqMap->setReturnValue('getCommand', 'SomeCommand');

        $front =& new FrontController_Partial();
        $front->expectOnce('createCommand', array());
        $front->setReturnReference('createCommand', $command);
        $front->expectOnce('getPrivate', array());
        $front->setReturnValue('getPrivate', true);

        $front->FrontController(..pass args if any to pass..); #!! call the constructor
        $front->processRequest($reqMap);
        $this->assertEqual($front->commandPath, '/modulepath/');
        $this->assertEqual($front->command, 'SomeCommand');
        $reqMap->tally();
        $front->tally();
        $command->tally();
    }
PS: I'll try and post some more later.

Posted: Tue Oct 04, 2005 3:01 pm
by Maugrim_The_Reaper
Some minor points. if you're using v1.0 of simple test, you need to pass $this to mock constructors. The latest alpha release simplifies things. You don't need to do that, or make tally() calls.
I'm actually using the alpha version - which is probably why it threw no errors when run. tally()'s removed...
Don't forget to explictly call the constructor of a partial mock.
As in $front->FrontController()?
Using mocks, we can define interfaces and (by setting expectations) describe the kind of behaviour we want them to exhibit.
Fine by me - I can see using abstract classes as the basis for generating full mocks - which is pretty much what DefaultCommand should be (all commands extend the abstract Command - not a real abstract in PHP4, but a stand in at least that'll cause an error if a method called on actual class has no such method overriding the parent).

Thanks for taking the time to set me straight...;)

Posted: Tue Oct 04, 2005 9:35 pm
by McGruff
I meant to take a closer look at the implementation which you've posted but I'm afraid I'm out of time for tonight - will get back to it tomorrow. One quick point: a FrontController fixture will need to set up examples of the "watched" var. This will either be something in $_GET or maybe you'll be getting the info from $_SERVER vars if you're using force type trickery. In order to avoid contamination between different test methods, or from other test cases in a group test run, you might do this:

Code: Select all

function setUp()
    {
        $this->_cached_get = $_GET;
        $_GET = array();
    }
    function tearDown()
    {
        $_GET = $this->_cached_get;
    }
Every test method will start off with an clean $_GET array - set values in the method itself, as needed. See also here.

The main thing is to clear the $_GET array before each test method. I've also saved and restored whatever GET vars were present at the start of the test run. This isn't likely to be a problem - I'm probably being too careful about making sure that the fixture leaves no tracks. As it happens, SimpleTest doesn't set any GET vars when you're running tests but you never know: it might in a future version. It wouldn't matter for a single test but might do for a group test run.

Posted: Wed Oct 05, 2005 3:00 am
by Maugrim_The_Reaper
I'll keep it in mind. For the moment the test requires no superglobals - since the mock RequestMapper has its returned values set to ad-hoc values. This is more likely to be tested in the RequestMapper test...

Posted: Wed Oct 05, 2005 11:04 am
by Maugrim_The_Reaper
After some more testing and some changes...

Code: Select all

<?php

require_once(APPBASE . 'System/Library/FrontController.php');
require_once(APPBASE . 'System/Library/RequestMapper.php');
require_once(APPBASE . 'System/Library/Abstracts/Command.php'); // Command abstract class (if PHP5 compliant)
Mock::generate('RequestMapper');
Mock::generate('Command');


class TestOf_FrontController extends UnitTestCase {

    function TestOf_FrontController() {
        $this->UnitTestCase('System => FrontController Test');
    }

    function testExecute() {
		$reqMap =& new MockRequestMapper();
		$reqMap->setReturnValue('getCommandPath', '/modulepath/');
		$reqMap->setReturnValue('getCommand', 'SomeCommand');
		$reqMap->expectOnce('getCommandPath', array());
		$reqMap->expectOnce('getCommand', array());

        $front =& new FrontController_TestExtend($reqMap); // see sub class below
        $this->assertEqual($front->privatePage, true);
		$this->assertIsA($front->requestMapper, 'MockRequestMapper');

		$front2 = $front; // FC copy used to simulate Intercepting Filter

		$command =& new MockCommand();
		$command->setReturnValue('getPrivate', true);
		$command->expectOnce('execute', array());
		$command->expectOnce('render', array());
		$command->expectOnce('getPrivate', array());

		$front->execute($front2, $command); // optional object passes (see test extend)
		$this->assertEqual($front2->commandPath, '/modulepath/');
		$this->assertEqual($front2->command, 'SomeCommand');
    }
}

// FC sub class for testing
class FrontController_TestExtend extends FrontController {
	
	function FrontController_TestExtend(&$requestMapper, $private=true) {
		parent::FrontController($requestMapper, $private);
	}

	// overridden methods to enable passing of mock objects to replace factory calls

	function execute(&$filterChain, &$command) {
		// skipped filterChain Factory call
		$filterChain->processRequest($command);
	}

	function processRequest(&$command) {
		$this->command = $this->requestMapper->getCommand();
		$this->module = $this->requestMapper->getModule();
		$this->commandPath = $this->requestMapper->getCommandPath();
		// here we skipped Command Factory call
		$this->checkPrivateFlags($command);
		$command->execute();
		$command->render();
	}
}

?>
After pushing some of the object creations out to Factory classes I wound up having to sub-class the FC to enable testing by replacing Factory calls with relevent Mock Objects. Unfortunately the optional parameter attempt fails. You can't both pass by reference AND set a default value - hence it looks like sub-classing is the only way to do this. Since processRequest() is now a private method, testing was pushed up into execute(). Think a few changes can be made to fix the IsA check...maybe...don't see it at the moment. Implementation follows...just the FC part since that's most relevant...

Code: Select all

<?php

require_once(APPROOT . 'System/Library/Abstracts/InterceptingFilter.php');
require_once(APPROOT . 'System/Library/RequestMapper.php');

class FrontController extends InterceptingFilter {
	
	var $module;
	var $command;
	var $commandPath;
	var $privatePage;
	var $requestMapper;

	function FrontController(&$requestMapper, $private=true) {
		if(!is_object($requestMapper)) 
		{
			trigger_error('Parameter [$requestMapper] is not a valid object', E_USER_ERROR);
		}
		$this->privatePage = $private;
		$this->requestMapper =& $requestMapper;
	}

	function execute() {
		require_once(APPROOT . 'System/Library/Factories/InterceptingFilterChain.php');
		$filterChain =& InterceptingFilterChainFactory::createChain($this);
		$filterChain->processRequest();
	}

	function processRequest() {
		$this->command = $this->requestMapper->getCommand();
		$this->module = $this->requestMapper->getModule();
		$this->commandPath = $this->requestMapper->getCommandPath();
		require_once(APPROOT . 'System/Library/Factories/Command.php');
		$command =& CommandFactory::createCommand($this->commandPath, $this->command);
		$this->checkPrivateFlags($command);
		$command->execute();
		$command->render(); // move tpl to Views for level 2 (or leave in Command for level 1)
	}

	function getPrivate() {
		return $this->privatePage;
	}

	function checkPrivateFlags(&$command) {
		$c_private = $command->getPrivate();
		if(!isset($c_private) || !is_bool($c_private)) 
		{
			trigger_error('The ' . $this->command . ' class has not defined whether this Command Object is private (requires authentication) or public (no authentication) - a ' . $this->command . '::privateCommand property must be set to a boolean value (TRUE or FALSE)', E_USER_ERROR);
		}
		if($this->getPrivate() !== $c_private) 
		{
			trigger_error('privatePage flag of FrontController does not match privateCommand flag of ' . $this->command . ' object - these must match for a Command to proceed', E_USER_ERROR);
		}
	}

}

?>
A few thoughts - seems there would be a lot of sub-classing required in TDD. You can't both pass an object by reference and set a default value. Also it would seem that PHP type hinting would have a similar effect. Presumably since Factory use may not be all that frequent it's just one of those extra tasks to bear in mind...:).

All the tests are passing for this...least till I break something again...;)

Not sure how this all applies under PHP5, presumably the use of type hinting (which I tend to like - being a strict C++ fan who moved to PHP) will create errors when a Mock Object (different type) is passed as a parameter that expects another class type (e.g. the parent abstract class type). That would seem to point to needing sub classes in at least some or more cases to facilitate unit testing. Could be wrong here - I am a newb afterall, and this has to have been addressed at some stage in the PHP TDD community...

Posted: Wed Oct 05, 2005 4:40 pm
by McGruff
I switched to PageControllers a while back. I've never implemented a presentable FrontController so I'm exploring this same as you.

How does this look:

Code: Select all

class FrontController extends InterceptingFilter {

    function FrontController() {
        $this->_filterChain =& $this->_FilterChain();
    }
    function execute() {
        $this->_addFilters();
        $this->_filterChain->processRequest();
    }
    function &_FilterChain() 
    {
        require_once(APPROOT . 'System/Library/Factories/InterceptingFilterChain.php');
        return new InterceptingFilter;
    }
    function _addFilters()
    {
        while(false !== ($item =& $this->_nextFilter())) {
            $this->_filterChain->add($item);
        }
    }
    /*
        return (object/false) the next filter or false when done
    */
    function &_nextFilter() 
    {
    }
    /*
        return (string) some meta data which specifies the chained objects
    */
    function _readConfigurationFile()
    {
    }
}
Actually, I've got some issues with InterceptingFilter but I'd better not go into that here. It is a common pattern.

I think you will need to get some info from the request URI. The FrontController needs to know which configuration to load. This is probably going to lead to a Request object.

Above I've speculated that a _nextFilter method could read a configuration file and then spit out filters, one by one.

Although the testing mantra is write the test first, then the code second, you do need to sketch the class out a little bit, either in your head or on paper. We know we want to call execute and make assertions about subsequent calls to other objects, and we also need to figure what assertions to make.

You could write the first test for the above without implementing _nextFilter or _readConfigurationFile. Knock them out with a partial mock, set a couple of return references for _nextFilter, and assert that the add() method for the (mocked) filter chain object is called the correct number of times, and that it receives the objects returned by _nextFilter as args.

Am I going in a direction you like?

Posted: Thu Oct 06, 2005 5:23 am
by Maugrim_The_Reaper
Actually, I've got some issues with InterceptingFilter but I'd better not go into that here. It is a common pattern.
I'm not even sure a InterceptingFilter is required... I added it on the assumption that post-processing (vs pre-processing) may be required. IMO I don't see any post-processing in the chain I have. So it's a common pattern that may not be solving a problem in this instance - hence it needs changing. I'll assume so...
I think you will need to get some info from the request URI. The FrontController needs to know which configuration to load. This is probably going to lead to a Request object.
At the moment this is passing around what I'm calling a RequestMapper (whether that's the pattern implemented or not is debatable - I doubt it, but the name describes the task). Basically it's parsing an "action" key passed in GET/POST to store values for:

Module; a directory name which groups Commands
Command; the name of the Command being mapped to
CommandPath; the full path to the mapped Command class.

I think this moving towards a Request object too. At the moment that InterceptingFilterChain includes an Input Filter which may be better off in a Request object. At the moment I use the GPC direct in the model layer (I have to deal with legacy code unfortunately - though an optional non GPC set of getters/setters can still be added for re-use).

In that case should the mapping to module/command values be in a Request object? hmm.. Or maybe I should be pushing this out to the CommandFactory to construct the mapping - passing in a Request object.

If its any help in context this would be a front to an MVC Lvl 1 design - the Commands are making calls to relevant Business Objects in the Model Layer which themselves delegate to the Data Access Layer where needed. Since it's Lvl 1, Commands currently retain responsibility for generating views with render() methods, though even a separated render() method has its complications...

Say, the IFC is taken out - where then to place User Validation and Session Expiry checks (they're use depends on whether the Frontcontroller was called with the $private flag set to true/false? More later no doubt...;)