MVC Refactor, Take 2

This is convention-over-configuration so where other formats are needed then configuration can be added.

I’ve got a folder structure where each model (Well, model API) has its own folder with models and templates, once a model has been selected, only templates in the directory relative to that model can be loaded. This has a nice added benefit of portability… want to copy a shopping basket from one site to another, just copy the /basket folder and it contains only the templates/models for the basket.

And this is the potential I am very much looking for. Up till now, application logic has not really been very portable, due to the coupling to the framework. I read this article trying to find solutions for my ideas and thought to myself, there has to be a better way. Although, there will always be some dependencies to a framework, I feel it is easier to inject and manipulate them in a model. And, it seems certainly much easier to standardize the model, than a bunch of controller actions.

Scott

I agree. The biggest advantage of the approach I suggested above is standardisation. The framework provides some interfaces and the developer programs around them, with a page-controller Web MVC set up, there’s no inversion of control, the application developer pulls in the required libraries/models/templates via service locators rather than having them provided via IoC, breaking Tell, Don’t Ask and LoD… resulting in less flexible code.

1 Like

I think we lost @Jeff_Mott? Jeff. What do you think of this direction?

I am on a short business trip. When I get back on Saturday, I’d like to try out the URL object mapping and formatting bit.

Scott

Still here. :slight_smile: Just been busy the past few days.

2 Likes

:smile:

Ahh, I was looking at it from the perspective of the router, where both approaches seemed to do essentially the same thing (instantiate a class then call a method that renders a template):

    $view = new SomeView('newThing.html.php');
    $response = $view->render();
    $controller = new SomeController();
    $response = $controller->someAction();

But it sounds like you’re actually talking about the overhead of writing an extra class, such as a controller class.

In Web MVC, if we truly had several wholly static pages (no user input, no database content), then we would make a generic controller. We might call it SimpleRenderController or something like that, and it would take the template to render as an argument from the router. Essentially we’d do it the way you’re doing it, but in Web MVC it’s not a rule to always do it that way. We would do it that way in cases where it helps (such as static pages).

The date and time examples are essentially also static pages (no user input, no database content), so in Web MVC, we’d probably use a generic controller for that too. So instead, let’s try some dynamic content (that is, database driven), which is by far the more common scenario in web applications. Let’s say there’s a template “thingList.html.php”, which we’ll reuse for “recently created list of things”, “recently viewed list of things”, and “search results list of things”.

First, a piece of code that both our approaches would use: a repository.

class ThingRepository
{
    public function getRecentlyCreated() { /* ... SQL */ }
    public function getRecentlyViewed() { /* ... SQL */ }
    public function search() { /* ... SQL */ }
}

And now we differentiate. In Web MVC, we would have three actions.

class ThingController
{
    // ...

    public function recentlyCreatedAction()
    {
        $things = $this->repository->getRecentlyCreated();
        return $this->templating->render('thingList.html.php', ['things' => $things]);
    }

    public function recentlyViewedAction()
    {
        $things = $this->repository->getRecentlyViewed();
        return $this->templating->render('thingList.html.php', ['things' => $things]);
    }

    public function searchAction()
    {
        $things = $this->repository->search();
        return $this->templating->render('thingList.html.php', ['things' => $things]);
    }
}

Contrasted with ToMVC, where we’d have three model classes.

class RecentlyCreatedModel
{
    // ...

    public $things;

    public function getThings()
    {
        $this->things = $this->repository->getRecentlyCreated();
    }
}

class RecentlyViewedModel
{
    // ...

    public $things;

    public function getThings()
    {
        $this->things = $this->repository->getRecentlyViewed();
    }
}

class SearchModel
{
    // ...

    public $things;

    public function getThings()
    {
        $this->things = $this->repository->search();
    }
}

If down the road the thingList template were to change (maybe it needs an extra argument), then I’d have to edit all three controllers and you’d have to edit all three model classes.

So here’s what I think I’ve concluded. For static pages, your approach is useful. Though, Web MVC can (and in real life does) use generic controllers to cater to this specific case. But for dynamic pages, which are by far the more common, the benefit of your approach seems severely diminished.

Depends a little on the specific circumstances. If the templates are related in some way (such as in our format example), then the controller can usually select the appropriate template through some string interpolation. But if we need to choose between several unrelated templates (seems unusual), then the code you showed is probably how we’d do it. In this scenario, your alternative approach would probably save a line of code or two (the one-line actions that invoke the common controller code). And we’d both need to add a route for each template.

Yes, that’s true for both of us. If I want my controllers to be able to use any of twig/blade/plain, or if you want your view classes to be able to use any of twig/blade/plain, and if those template engines don’t have compatible interfaces, then in both our approaches, we’d have to write adapter classes.

Web MVC doesn’t force us to use (or not use) either service locator or dependency injection. If you want to be able to substitute the model, then you can still take it as a constructor argument and let a dependency container construct the controller.

The problem with this approach, of course, is if the template needs some information from a model (See my clock example using clock.html.php and date.html.php). There’s no way you can do this using a generic controller because in web mvc the controller passes the data (in this case the date/time) to the model. You have to write a controller to achieve this. There are lots of cases where there is a read but no write: Displaying the latest 10 news stories, displaying a list of “new arrivals” products, displaying “on sale” products. All of these read from the database but require no interaction from the user.

You’re making a distinction between dynamic and static that isn’t really correct. There are actually 3 different cases:

  1. Static pages which are just rendering a template (e.g. Thank you for your enquiry we will get back to you shortly). You can achieve this in web mvc and tomvc with a generic controller

  2. Pages which display some information from the model/database in a predefined format e.g. latest news, latest products, special offers (which might use the same template as latest products!). You cannot achieve this in webmvc with a generic controller, with tomvc you can

  3. Pages which display some information from the model/database and allow the user to manipulate how it’s displayed (forms, search results, pagination, loading a specific record into a template e.g. viewing a product by ID)

Of course, but the point you were making is that by adding a ‘view’ I didn’t have one less class in the system:

Once you account for your template wrapper, ToMVC does have that one fewer class. If I want to support other template systems I add: TwigView, BladeView etc.

In which case, where is the decision made about which model to inject? If it’s in the router then my point from earlier still stands: If the router selecting the model and the controller, what is the problem with having it also select the view? If as you say it’s in the DIC, have the DIC also construct a view.

The problem with putting it in a DIC is that the configuration is spread among two places and potentially complex. Consider the clock example with two models that supply the same data from multiple sources. You need to now route to the DIC and fetch two differently configured instances of the controller on different routes, one for the NTP version and one for the standard version. This makes the router far more complex, something you were trying to avoid previously.

There is a simple conceptual issue with this and with a “true” MVC architecture. Normally, the controller in MVC has no interaction with the view. The controller is there purely for the manipulation of the model. The view then always calls the model to get the modified state.

The issue we have with web technology and MVC is, the actual “view” is in the web browser. On top of that, the “commands” needed for the controller are also coming from the web browser. There is a “conversion” to HTTP (or a web socket) required between the server and the browser. This interface simply needs to convert the HTTP request, run it through the MVC to get a response. Then the converter must send the response back. This is exactly what my example does.

Um, there is user input in my mini-MVC example. And it wouldn’t be hard at all to add a database layer to my example either. In fact, I’d say it is easier and more logical to add the database layer in this type of web MVC, now that all the data MUST be manipulated in the model (like it should be).

Scott

There’s another big problem with Web MVC as well: It ignores the fact there is such a thing as application state. It understands domain state but makes no attempt to account for the state of the application. As such, the application state is stored in the controller action in a very transient way.

Application state is things like:

  • What is the ID of the record being viewed/edited
  • Was the form submitted or not?
  • Is the submitted data valid?
  • Which page of results are we on?
  • What is the filter for the current set of results? e.g. search term, search order, category filter

The problem with storing this in temporary variables in the controller is that the controller has to give this information to anything that needs it and there’s no way of supplying the application state from anywhere else. Using a proper MVC approach where the state is in the model, the model can read the state from anywhere.

To be clear, here is how Web MVC would probably handle your clock/date scenario.

class GenericClockController
{
    public function doAction($clockObj, $template)
    {
        return $this->templating->render($template, ['time' => $clockObj]);
    }
}
if ($_SERVER['REQUEST_URI'] === '/clock') {
    $response = new GenericClockController()->doAction(new Clock(), 'time.html.php');
} else if ($_SERVER['REQUEST_URI'] === '/date') {
    $response = new GenericClockController()->doAction(new Clock(), 'date.html.php');
} else if ($_SERVER['REQUEST_URI'] === '/clock-ntp') {
    $response = new GenericClockController()->doAction(new NtpClock(), 'time.html.php');
} else if ($_SERVER['REQUEST_URI'] === '/date-ntp') {
    $response = new GenericClockController()->doAction(new NtpClock(), 'date.html.php');
}

Like I said in my last post, we’d do it essentially the way you’re doing it. I do agree this is the better approach for this specific scenario. To be clear, this scenario is static content with multiple variations. The reason you don’t see this kind of implementation very often in Web MVC applications is because this scenario doesn’t actually occur very often. Static pages are already few and far between (maybe a terms of service page, an about us page, things like that), and multiple variations of those pages almost never happens in real life (excluding i18n, which is typically handled in a completely different way). So, again, to be clear, Web MVC can and does shift to your kind of solution when it’s helpful to do so, but it usually isn’t.

Didn’t I cover exactly this with the “recently created”, “recently viewed”, and “search” list of "thing"s??

Again, this case describes the “list of things” that I gave example code for in my last post. In Web MVC, we’d make three controllers, and in ToMVC, you’d make three model classes. Both the controllers and your model classes would be coupled to the template.

The Template class was obviously generically named, but what it is is a plain PHP template rendering engine. It could have just as easily been, for example, a Twig template rendering engine. But what you did in your examples was remove the rendering engine and instead bake the template rendering logic straight into your view class. In real life, or even if we just expanded our examples a little further, you’d wouldn’t have been able to get away with that. Instead, your view classes should use a template rendering engine.

The dependency container config would define a variety of controller services, and the router would pick a particular service.

Because you’d be limiting your view selection options for no good reason. For example, here’s sample code from Fowler’s book:


He chooses between a couple templates based on what he gets back from the database. But if the router tells you which template you’re supposed to use, then you don’t get this kind of freedom.

I was talking about Tom’s date/time examples.

Well, I did make my example after his. There is a select in Tom’s example, where the user can select the time zone.

Scott

But that’s display logic based on a result. Put the if statement in the template. Chances are, some of the information about a Classic Album and an Album is the same (or we wouldn’t be using the same repository) so we can just use the if to dictate which version to show. The same is true of “Not Found”. This is clearly display logic “If no album is found, display an error” so should go in the view (the template).

By doing this, we can make a generic controller. I’ll use the same album example. The framework could supply a generic controller and interface:

class SingleEntityController {

	public function __construct(EntityModel $model) {
		$this->model = $model;
	} 

	public function get($id) {
		$this->model->load($id);
	}
}


interface EntityModel {
	public function load($id);
}

The developer would then provide the model and template:

class AlbumModel implements EntityModel {

	public function __construct(AlbumRepository $repository) {
		$this->repository = $repository;
	}

	public function load($id) {
		$this->entity = $this->repository->findById($id);
	}

	//For the template to call..
	public function getEntity() {
		return $this->entity;
	}
}

Ok it’s half a dozen of one and six of another, but wait! That can be simplified can’t it, I’d have to write a model for Albums, Blogs, Cars, Products, etc. Consider this:

class RepositoryModel implements EntityModel {

	public function __construct(Repository $repository) {
		$this->repository = $repository;
	}

	public function load($id) {
		$this->entity = $this->repository->findById($id);
	}

	//For the template to call..
	public function getEntity() {
		return $this->entity;
	}
}

Now I can write a route that looks like this:

$model = new RepositoryModel(new AlbumRepository());
$controller = new SingleEntityController($model);
$view = new View('album.html.php', $model);

Or like this:

$model = new RepositoryModel(new BlogRepository());
$controller = new SingleEntityController($model);
$view = new View('blog.html.php', $model);

or

$model = new RepositoryModel(new ProductRepository());
$controller = new SingleEntityController($model);
$view = new View('product.html.php', $model);

In the example you provided, because there are domain concepts in AlbumController (is it an album or a classic album, does it exist or not?) as well as hardcoded template names, this kind of flexibility just isn’t possible and requires a new controller with similar logic for each and every type of domain entity.

edit: The difference between the two approaches is entirely about IoC.

All of this information(except for “is valid”) is stored in the request object. Why would anyone think it’s stored in the controller?

1 Like

It doesn’t seem that way at all to me. In the Web MVC examples, the repository and template rendering objects are still being injected. But in your examples, you’re not just abstracting away the objects themselves, you’re trying to abstract away the invocations we make on those objects, and trying to abstract away the arguments we pass to the abstracted invocations. Just because Web MVC doesn’t go to that extreme doesn’t mean its dependency objects aren’t still injected.

Chances are, sure, but not guaranteed. The two might actually be very different that we prefer to keep them as separate templates. But your approach that you advertise as more flexible doesn’t give us this choice.

Compared to Fowler’s original album = Album.find(id), your alternative strikes me as significantly more complex. I hope we get a lot of bang for this buck.

This relies on the very big assumption that albums, blogs and products all use the exact same logic. But one might load by ID while another loads by slug, or another might want to load using a pre-loading query, any of which would foil your reusability and force you to make separate model classes. Or one might be loading a single entity but still have other input to consider such as session data, which would force you to make separate controller classes.

I’m tempted to call your approach “premature refactorization”. You start with the assumption that certain chunks of code will be identical, then refactor as appropriate. But they often won’t be identical, and your prematurely refactored code ends up making things more rigid rather than more flexible.


I feel like this topic is almost talked out. I think the code comparisons were a lot better than our usual hand wavy claims; hopefully others thought so too. But obviously I’m not sold.

I’ll let you have the last word.

This is exactly my thinking too and why the request in my example MVC is passed on to the view, controller and model. The information needed to get all jobs done is in the request. However, the request is just the returned/ modified answer from the previous response, which was formed by the model. The view (the actual response) is only a representation of the model, be it HTML, JSON, PDF, etc. When you talk in “page controller terms”, you simply can’t say that really. The response in web MVC is a mixed representation of model and in some cases, too much controller logic (the dreaded fat controller). In other words, having to figure out what representation/ response needs to be sent in the controller with which model means a single controller class automatically breaks SRP. It is also why controllers in web MVC in the popular frameworks are rarely portable and hard to test.

Scott

If you ask me, the template selection excerpt of Fowler’s code belongs in the view, in the main “Album” template itself. So, depending on what data and state the model holds, the proper representation would be selected. This controller class is also clearly breaking SRP, IMHO. If the template decision needs to change, like a third album type is added, this controller would need to change. If the model source needs to change, I’d need to change this controller too. Two reasons for change at two different times = broken SRP. Granted, the model source probably won’t change at all. But, that is beside the point.

Scott

I realize now, I missed the most important part in my post above. The client clock model class. Doh! :open_mouth:

I am almost finished with including parameters in the URL to control anything the client wants. I’ll post everything again anew.

Scott

You’re constructing a template with a hardcoded filename. Just because it’s not a class doesn’t mean it’s not the same as new Template(). The same is true of a model, in most web mvc implementations the model is chosen using a service locator, again, going against IoC because the decision of what to include is being made in the controller, which means that the controller logic cannot easily be used with another template.

That’s just not true though is it? there’s no reason the template can’t do this:

if ($album instanceof ClassicAlbum) include 'classicalbum.html.php'
else include 'album.html.php';

This is a lot better than having the logic in the controller because the same controller can be used with this template or another template that does something entirely different.

That’s rather disingenuous. If we’re doing an apples to apples comparison it’s the model I provided vs fowler’s controller. In fowler’s example, the developer would need to write the entire controller, in my first example the developer needs to write a slightly shorter model. In my second, the developer has to write only the router configuration.

Which is true. Let’s say one of the templates needs some session data. If that’s the case you need to write a whole new controller action that loads the template and passes the session. Let’s start with fowler’s example (ported to php)

class AlbumController {
	
	
	public function doGet() {
	    $album = Album::find($request->getParameter('id'));
	    if ($album === null) $this->template->render('missingalbumerror.tpl.php');
	    else {
	    	if ($album instanceof ClassicAlbum) $this->template->render('classicalbum.tpl.php', ['album' => $album]);
	    	else $this->template->render('classicalbum.tpl.php', ['album' => $album]);
	    }

	}

}

Now from this base point, suppose I want a different template which displays something from the session and the album. I now need to add another function in the controller:

class AlbumController {
	
	
	public function doGet() {
	    $album = Album::find($request->getParameter('id'));
	    if ($album === null) $this->template->render('missingalbumerror.tpl.php');
	    else {
	    	if ($album instanceof ClassicAlbum) $this->template->render('classicalbum.tpl.php', ['album' => $album]);
	    	else $this->template->render('classicalbum.tpl.php', ['album' => $album]);
	    }

	}


	public function doGetWithSession() {
	    $album = Album::find($request->getParameter('id'));
	    if ($album === null) $this->template->render('missingalbumerror.tpl.php');
	    else {
	    	if ($album instanceof ClassicAlbum) $this->template->render('classicalbum.tpl.php', ['album' => $album, 'session' => $_SESSION['whatever']]);
	    	else $this->template->render('classicalbum.tpl.php', ['album' => $album, 'session' => $_SESSION['whatever']]);
	    }

	}

}

Admittedly, it would be possible to refactor the class and move the repeated logic into its own method… but this takes time!

On the other hand, with my approach, there is no need for this refactoring, you’d add the model:

class AlbumRepositoryModel extends RepositoryModel {
	public function getSession() {
		return $_SESSION['whatever'];
	}
}

and add the route accordingly to pull in the new template and the new model.

Alternatively if you need to load via a URL slug or another field in the database then it’s simple enough to make a generic model and controller to handle it (If it’s a common function like that).

I’m not sure I agree with this. Having used this approach for several years now (After migrating away from Web MVC as I found myself repeating code far too frequently and having to write far too much code to do simple tasks). With controllers for: Forms, Result sets (With extensions for paginate, search) and single entities, this handles 90%+ of my use-cases. On the odd occasion I do have to write a controller but in web mvc I’d have had to have written that controller anyway, and I’d also have had to have written it the other 90% of the times where I didn’t need to.

One last question if you don’t mind: I feel that this topic has been me explaining the advantages of a more mvc-like architecture and you mostly saying “You can do that in WebMVC too if you make these changes”, what I would like is a brief list of what you think the advantages are of web mvc over my suggested approach.

I’ll do the same for my approach:

  • Have to write significantly less code when just viewing a static template
  • Have to write significantly less code when loading data from a model without any user input
  • Have to write less code for common use cases (Forms, pagination, loading a single entity by ID)
  • Have to write the same amount of code otherwise
  • The model, view or controller can easily be substituted without rewriting the controller action
  • Which overall significantly lowers development time
  • Programming around interfaces gives a much more standardised approach so it’s easier for new developers to understand
  • Controllers don’t break SRP
  • Controllers don’t break encapsulation (Knowing what response type the view is)
  • Controllers don’t break LoD/Tell, Don’t ask
  • Looser coupling overall (Controllers are tightly coupled to models and templates in web mvc most of the time)

Ok, here goes nothing #2. This is going to start over, explaining my idea for MVC without a router. It also entails the “what ifs” Tom threw at me like what if the format needs to be different or the data needs to come from somewhere else. So I made a parameter builder. I’ll explain later.

Ok. Again, our entry is a front controller like Symfony’s.

app.php

<?php

/*
 * Generic entry point/ front controller.    
 */

require_once('HTTPConverter.php'); #


$httpConverter = new HTTPConverter();
$request = $httpConverter->createRequest();
$response = $httpConverter->handleRequest();
$httpConverter->sendResponse();

We call HTTPConverter.php

HTTPConverter.php

<?php


/*
 * A converter to convert HTTP to a request object and handle it to get a response
 *
 */

require_once ('Request.php');
require_once ('View.php');
require_once ('Controller.php');
require_once ('Model.php');

class HTTPConverter
{

    private $request;
    private $response;
    private $controller;
    private $model;


    public function __construct()
    {

        // prepare anything necessary

    }


    public function createRequest()
    {
        // create the request object
        $this->request = new Request();

    }

    public function handleRequest()
    {
        $this->model = new Model($this->request);

        // if the request is a write request (i.e. a post, delete or put), do the controller first
        if ( $this->request->isWrite() ) {
            $this->controller = new Controller($this->request, $this->model);
        }

        // you'll always need a response, so call the view to get it.
         $this->response = new View($this->request);
    }

    public function sendResponse()
    {
        echo  $this->response->render($this->model);

    }

}

This controls our MVC entry and exit points with request and response.

Like in the diagram I presented earlier.

We need the request object.

Request.php

<?php

/*
 * Create the Request object from global variables.
 */

class Request
{
    public $object;
    public $method;
    public $path;
    public $postData;
    public $urlParams;
    public $urlParamCount;

    public function __construct()
    {
       $this->setObject();
       $this->setURLParams();
       $this->filterPostData();
       $this->setMethod();

    }

    private function setObject()
    {
        $urlElements = explode('/', $_SERVER['REQUEST_URI']);
        $this->object = $urlElements[1];
    }

    private function setURLParams()
    {
        $urlElements = explode('/', $_SERVER['REQUEST_URI']);
        // because the object is always the first element and we already define its own variable, drop it
        $this->urlParams = array_slice($urlElements, 2);
        $this->urlParamCount = count($this->urlParams);


    }


    private function filterPostData()
    {
        //no filtering yet, but you get the point
        $this->postData = $_POST;
        return $this->postData;
    }

    private function setMethod()
    {
        switch ($_SERVER['REQUEST_METHOD']) {
            case 'POST':
                $this->method = 'update';
                break;

            case 'PUT':
                $this->method = 'create';
                break;

            case 'DELETE':
                $this->method = 'delete';
                break;

            case 'GET' :
                $this->method = 'read';
                break;

            default: 'read';
        }
    }

    public function isWrite()
    {

        return $this->method == 'update' OR $this->method == 'create' OR $this->method =='delete' ? true : false;

    }

    public function isRead()
    {
        return $this->method == 'read'? true : false;
    }

}

I added some logic to create the urlParams. They will be decisive in the later logic.

Now our first call to our application is only a read, so let’s show the view only.

View.php

<?php

/*
 * View class for creating an output
 *
 */

class View {

	private $template;
	private $model;
        public $headers;


	public function __construct(Request $request)
        {
            $this->template = $request->object . ".html.php";
	}

	public function render($model)
	{
            $headers = [];
            ob_start();
            require ( "./templates/".$this->template );

            return  ob_get_clean();
	}
}

In our HTTPConverter, we had first called on the initial model, which is all we need to render.

We have currently two choices (the parameter mapping needs to be expanded for more flexibility I know, but humor me on this). We can just call “/clock” or we can now call “/clock/ntp/date-only”. The initial result is the same.

Now we make a selection and need to go through a controller for the update and update the model.

Controller.php

<?php

/*
 * Our very, very, very thin controller! The thinnest controller ever!!!
 *
 */

class Controller {

	private $model;
        private $request;

	public function __construct(Request $request, Model $model) {
                $this->request = $request;
		$this->model = $model;
                $this->decideAction();
	}


        private function decideAction()
        {
            switch ($this->request->method){
                case "create":
                    $this->model->create();
                    break;
                case "update":
                    $this->model->update();
                    break;
                case "delete":
                    $this->model->delete();
                    break;
            }

        }

}

Here is our model.

Model.php

<?php

/*
 * A class for our model (building)
 *
 */

require_once('ClientModelFactory.php');



class Model {

	public $violations = [];
        private $request;
        public $clientModel;

        public function __construct(Request $request)
        {
            $this->request = $request;
            $this->buildClientModel();

        }

        public function buildClientModel()
        {
            $this->clientModel = ClientModelFactory::build($this->request);
            return $this->clientModel;
        }

	public function create()
	{
	    // create something new
	}

        public function update()
	{
	    $this->clientModel->update($this->request->postData);
	}

        public function delete()
        {
            // delete somthing
        }

}

And the ClientModelFactor.php

<?php

/*
 * A stimple factory to build our clientModels
 *
 */


class ClientModelFactory
{
    public static function build(Request $request)
    {
        $class = (ucfirst($request->object))."Model";
        require_once ("models/". $class .".php");
        if (class_exists($class)) {
            return new $class($request);
        }
        else {
            throw new Exception("Invalid object given.");
        }
    }
}

The trick now is the added logic to the client’s ClockModel.php, where the client creates all the necessary logic. This is actually where I want the parameters (the configuration) too, as it is ALL in one place. No crazy config files needed really. But, we can discuss the need, as there will be some for sure. At any rate, what was missing in my above example was the client’s model.

ClockModel.php

<?php

/*
 * Our clock model class
 *
 */

require_once('ObjectParameters.php');


class ClockModel {

    private $request;
    public $timezones = ['Europe/London' => 'London',
                          'America/New_York' => 'New York',
                          'America/Los_Angeles' => 'Los Angeles'];

    // this is the URL structure. The first elements is always the object, everything after the object will be your object parameters
    // we'll need to come up with a more flexible method later
    public $urlPathMap = '/clock/dataSource/dateFormat';

    // since you have them in your path map above, you must also declare the variable defaults! This should also be automated!
    public $objectParams = ['dataSource' => 'default',
                                   'dateFormat' => 'default'];
    public $timezone;
    public $submitted = false;
    public $time;

    public function __construct(Request $request)
    {
        $this->request = $request;
        $this->getObjectParams();
    }

    private function getObjectParams()
    {
        $objectParameters = new ObjectParameters($this->request, $this->urlPathMap, $this->objectParams);
        $this->objectParams = $objectParameters->parameters;
    }

    public function create()
    {
        // create something new
    }

    public function update($postData)
    {
        if ( $this->isValid($postData) ) {
            $this->submitted = true;
            $this->timezone = $postData['timezone'];
        }
    }

    public function delete()
    {
        // delete something
    }

    public function isValid($post)
    {
        return (null !== isset($post['timezone'])) ? true : false;
    }

    public function getTime()
    {   //var_dump($this->objectParams->parameters);
         if ( $this->objectParams['dataSource'] === 'default' ) {
            $timezone = new DateTimeZone($this->timezone);
            $date = new DateTime;
            $date->setTimeZone($timezone);
            return $date;
        }elseif (  $this->objectParams['dataSource'] === 'ntp' ){
            $date = $this->getNTPTime();
            return $date;
        }
    }

    public function getNTPTime()
    {
        // go and get the time from some NTP server (for now just do the same as default as an example)
        $timezone = new DateTimeZone($this->timezone);
        $date = new DateTime;
        $date->setTimeZone($timezone);
        return $date;
    }

    public function getCity()
    {
        return $this->timezones[$this->timezone];
    }

    public function success()
    {
            $this->submitted = true;
    }
}

I realize this client model is getting very fat. But, it wouldn’t be at all hard to refactor and make some new sub-objects, like for the different tasks, like UpdateClockModel, DeleteClockModel, CreateClockModel.

The new part I’ve added it the Object Parameters object.

ObjectParameters.php

<?php

/*
 * This is where we setup the object parameters.
 */

class ObjectParameters
{
    public $parameters = array();
    private $request;
    private $clientURLPathMap;
    private $parameterKeys;
    public $defaultObjectParameters;

    public function __construct(Request $request, $urlPathMap, $defaulObjectParameters )
    {
        $this->request = $request;
        $this->defaultObjectParameters = $defaulObjectParameters;
        $this->clientURLPathMap = $urlPathMap;
        $this->setParameterKeys();
        $this->buildParameters();
    }

    private function setParameterKeys()
    {
         $mapElements = explode( '/',$this->clientURLPathMap);
         $mapLength = count($mapElements);
         $this->parameterKeys = array_slice($mapElements, 2, $mapLength - 1);

    }

    public function buildParameters()
    {
        // this is in flexible, as we can't set up for different parameter needs. Need to refactor.
        if ( $this->request->isRead() AND $this->request->urlParamCount === count($this->parameterKeys) ) {
            for($i=0; $i < $this->request->urlParamCount; $i++) {
                $this->parameters[$this->parameterKeys[$i]] = $this->request->urlParams[$i];
            }
       }else{
           $this->parameters = $this->defaultObjectParameters;
       }

       // this is ugly. Refactor later. But, needed to set up hidden fields for our parameters.
       if ( $this->request->isWrite() ) {
           foreach( $this->request->postData as $key => $value ) {
               foreach ( $this->parameterKeys as $parameterKey ) {
                   if ( $key === $parameterKey ) {
                       $this->parameters[$parameterKey] = $value;
                   }
               }
           }
       }
    }

}

I am not too proud of this class, as I know it is mofugly code. But, I hope it gets the point across at what I am attempting to accomplish.

Edit: Of course, it also works.

:blush:

Scott