MVC vs PAC

OOP doesn’t mean that using methods to consolidate repetition within a class suddenly became a faux pas. We’re getting back into yu-huh/nu-uh territory, so I’m going back to my original request: Please provide step by step refactoring that illustrates your case. When you (if you) do so, please approach it as if you were reporting a software bug. Start by showing a dead simple example, as simple as you can make it, that demonstrates the issue, then show your step by step refactoring to resolve that issue.

That “dodgy workaround” would be calling a different method to get a different result. Dodgy? And while I would have to edit the controller code, you too would have to edit your WireItUp code.

Only one need be coded in Symfony as well, so… no difference. Remember earlier how straightforward it was to isolate the data fetching logic? Just because your idea of using Symfony is to copy-paste doesn’t mean that’s how it’s actually done.

Ditto in Symfony. Remember that we have the option of injecting dependencies into our controllers. So we could have two doctrine objects to represent the two databases and inject them respectively into two instances of the same controller class.

Your logic is starting to make less and less sense to me.

Yes, templates are supplied data. That’s how PAC works. No, that does not make them pointless. Yes, displaying data means you must fetch data. No, you do not have to repeat data fetching logic.

If you’re going to continue criticizing Symfony, then I highly recommend you first build something in Symfony (and ask questions), because, frankly, you don’t seem to have any clue how it works.

As I have said, my issue is not with Symfony in particular. I have no interest in using it because the PAC architecture is problematic. I’ve still yet to see you attempt to answer the question “Why is PAC better than MVC?”

OOP doesn’t mean methods suddenly became off limits. We’re getting back into yu-huh/nu-uh territory, so I’m going back to my original request: Please provide step by step refactoring that illustrates your case. When you (if you) do so, please approach it as if you were reporting a software bug. Start by showing a dead simple example, as simple as you can make it, that demonstrates the issue, then show your step by step refactoring to resolve that issue.

Unfortunately I had hoped anyone participating in a discussion at this level would have an understanding of the difference between procedural and oop and understand the problems created by introducing dependencies in code.

Let’s take a step back, away from frameworks, away from everything back to the fundamental difference between OOP and Procedural programming.

Let’s create a basic function that formats a unix timestamp as a date using a specified format:


function formatDate($format, $timestamp) {
	return date($format, $timestamp);
}


$formattedDate = formatDate('d/m/Y', time());

In OOP this would be expressed as:


class Date {
	private $timestamp;

	public function __constuct($timestamp) {
		$this->timestamp = $timestamp;
	}

	public function format($format) {
		return date($format, $this->timestamp);
	}
}

$date = new Date(time());

$formattedDate = $date->format('d/m/Y');

OOP is more code, but there is a key difference: encapsulation. What this means is that the part of the code which is concerned with formatting the date is not concerned with locating the timestamp.

As such, the constructed $date object can be passed around the system with along with its state.

The procedural method is stateless this means it needs to be passed all its parameters every time. The knock on effect here is that anywhere that calls the formatDate() function has the responsibility of providing both the timestamp and the date format.

A procedural template would look like this:


<p>The date is: <?php echo formatDate('d/m/Y', $timestamp); ?></p>

Whereas the OOP version would look like this:


<p>The date is: <?php echo $date->format('d/m/Y'); ?></p>

The practical difference here is minimal, isn’t it? In both cases, a single variable has been bound to the template.

However, from a flexibility perspective, it’s not great. formatDate() can only take timestamp as a parameter. What if I had a system where some of the dates were in a string format of ‘YYYY-DD-MM’?

With the procedural method, I’d have to create a stringToTimestamp() method like so:


function stringToTimeStamp($string) {
	list($y, $m, $d) = explode('-', $string);
	return mktime(0, 0, 0, $m, $d, $y);
}

And every time I came across a date, I’d have to convert it:


<p>The date is: <?php formatDate('d/m/Y', stringToTimeStamp($strDate)); ?></p>

The problem? I had to alter the template* to know that the date is coming in as a string not a timestamp.

  • Yes, this could be done when it’s assigned to the template, but that moves the problem to the binding logic. The conversion must be done somewhere.

OOP avoids this problem entirely:


class StrDate {
	private $string;

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

	public function format($format) {
		list($y, $m, $d) = explode('-', $this->string);
		return date($format, mktime(0, 0, 0, $m, $d, $y));
	}
} 

I can now pass a StrDate object to the template and the OO template works without being changed, as does the code which is binding variables to the template.

Hopefully that’s a simple enough demonstration of OOP vs Procedural. Now, back on track… your code is procedural and not oop not because it’s using methods, but because your class is self-configuring and stateless.

Consider this:


class Date {
	private function getCurrentDate() {
		return time();		
	}


	private function format($date, $format) {
		return date($format, $date);
	}


	public function output() {
		return $this->format($this->getCurrentDate(), 'd/m/Y');

	}
}

[b]This is the important part: This is functionally identical to the BlogController you posted: The state is generated internally on the fly every time output() is called and passed around internally, it never truely exists beyond the life of a call to $date->output(). This class can only ever output the current date. Compare this to your BlogController and at this basic level it’s identical: The class is self-configuring and has no real state.

Again, this is a very important distinction. If you cannot understand this point, then it’s not worth continuing this discussion at all.
[/b]

Hopefully, this is a clear enough example. Now, this is procedural because it is identical to:


function getCurrentDate() {
	return time();		
}


function format($date, $format) {
	return date($format, $date);
}

function output() {
	return format(getCurrentDate(), 'd/m/Y');
}

The problem this causes is quite large. Going back to the non-oo date class: How can I substitute the getCurrentDate() function?


class Date {
	private function getCurrentDate() {
		return time();		
	}


	private function format($date, $format) {
		return date($format, $date);
	}


	public function output() {
		return $this->format($this->getCurrentDate(), 'd/m/Y');
	}
}

Because it’s calling $this->format() internally it will ALWAYS call Date::getCurrentDate(). $this will always resolve to a Date object.

The only way this can be achieved is inheritance. I don’t want to change the code because it’s in use elsewhere and needs its current implementation.

So…


class TomorrowsDate extends Date {
	private function getCurrentDate() {
		return strtotime('+1 day');	
	}

	
}

This works, doesn’t it?

What if I want to change the date format in the output function for a specific instance of TomorrowsDate (Not all of them, mind!), I have to extend, again:


class TomorrowsDateAsMySql extends TomorrowsDate {
	public function output() {
		return $this->format($this->getCurrentDate(), 'Y-M-D');
	}
}

But what if I want a normal date class formatted as MySQL… now I need to extend the base Date class again!


class DateAsMysql extends Date {
	public function output() {
		return $this->format($this->getCurrentDate(), 'Y-M-D');
	}
}

And there we have repeated code! And a very messy, hard to understand class inheritance hierarchy.

The problem with this, and your controller examples is that the objects have no real state. They’re self-configuring and will only ever get configured in a very specific way. Their transient state only exists for the duration of one method and not beyond that, and because the state is dictated internally, it makes it impossible to substitute parts of the system.

Now imagine if we were’t using $this in the non-oo Date class but intead calling a method on another class and the pieces should fit together.

I hope that’s simple enough. I really can’t make it any simpler. It really is a case of OOP vs procedural.

After reading that back I’d like to clarify, the biggest issue is with $this. Because it can only ever resolve to a very specific implementation, it is from a theoretical point of view, static. There’s no way to substitute the meaning of $this at runtime. With any other variable, such as $this->foo, the meaning of $this->foo can be substituted at any time, it can point to anything. $this cannot, it must always resolve to an instance of the current class. And that is why it is, for practical purposes, procedural. $this->query() locks you into a specific implementation just as much as mysqli_query does. Whereas using $this->db->query(); gives you access to whatever query method is provided by whatever $this->db happens to be… and that can be any class at all, it’s not confined to a single implementation.

So is it now a blunder for a class to ever call one of its own methods? Is $this->anyOtherMethod() also bad? I had suggested that a class could create a private method to consolidate repetitious code found within itself. Apparently this earned me TomB’s Doesn’t Understand OOP trophy.

Not at all, calling internal methods is fine, but you have to understand that it’s going to call a very specific implementation. This is usually desirable behaviour. As with anything in programming, there’s a time to use them and a time to avoid them. If it would ever be beneficial to substitute that method for a different implementation, then using $this->method() is not the best candidate for the job. Much as Class::method() isn’t.

Thank you for that acknowledgement. From post #20 onward, you seemed to hold the opposite opinion.

When you’re using it to lock yourself into a specific configuration, I most certainly do hold the opposite opinion. As I said, there’s times to use things and times to avoid them. Using $this->method() locks you into a very specific implementation, it’s up to you as a developer to decide whether that’s a problem or not. In the context of a method that you may want to substitute for something else, that is definitely a problem.

edit: As a rule of thumb, if the contents of a method could be copy/pasted into the places it’s called and the class would be just as useful it’s fine. As well as if the method is public- if the outside world can use the object in this manner, using it internally will make no difference.

And what configuration did I lock myself into when I suggested the private method to consolidate repetitious code?

EDIT:

As a rule of thumb, if the contents of a method could be copy/pasted into the places it’s called and the class would be just as useful it’s fine.

I’m pretty sure my private method suggestion was exactly such a case.

Look at my Date example, it’s analogous to this:


class BlogController extends Controller
{
    // ...

    private function renderList($items)
    {
        return $this->render(
            'AcmeBlogBundle:Blog:list.html.php',
            array('posts' => $items)
        );
    }

    private function getPostsByUserId($userId)
    {
        $posts = $this->get('doctrine')->getManager()
            ->createQuery('SELECT p FROM AcmeBlogBundle:Post p WHERE p.userId = ' . $userId)
            ->execute();

        return $posts;

        // A little further into the refactoring process,
        // and Symfony projects would actually move this data fetching code
        // into a PostRepository class
    }
}  

Here, renderList() can only ever use that very specific implementation. You cannot use a different template, for instance. getPostsByUserId() cannot be substituted at runtime either. The suggestion of a “PostRepository” would fix this, but you’d call that from the individual methods, not via the getPostsByUserId() method.

The problem is that it’s self-configuring. Your class cannot be configured in any other way because it’s stateless.

That private method is indeed, but the private methods in your example code were not.

Looking once again at your WireItUp code, you’re also “locked” into a specific implementation. Your WireItUp hardcodes the template name (postList.tpl) as well as the data fetching logic (EveryPostList).

I think you misunderstand. That code could potentially supply anything at all because nothing is tightly coupled. It doens’t matter where the configuration comes from, be it a static string passed directly or somewhere else. Calling $this->getSomeConfiguration() means that the configuration can only come from the getSomeConfigruation() method of the specified class. It’s loose vs tight coupling.

Indeed. Your WireItUp code can use whatever template it wants and it can instantiate whatever data fetching object it wants. Likewise, a framework controller can render whatever template it wants and fetch whatever data it wants.

What am I missing?

Ok, before we continue: back to basics, do you agree that the date class I provided, is not OO because it is stateless, it can only ever be configured in one very specific way and changing that configuration requires some very ugly workarounds?


class Date {
    private function getCurrentDate() {
        return time();        
    }


    private function format($date, $format) {
        return date($format, $date);
    }


    public function output() {
        return $this->format($this->getCurrentDate(), 'd/m/Y');

    }
}  

The problem:

-It’s impossible to create a Date object which represents anything other than current date because the class configures itself.

There is nothing here that makes use of OOP and it can equally be expressed with the code:


function getCurrentDate() {
    return time();        
}


function format($date, $format) {
    return date($format, $date);
}

function output() {
    return format(getCurrentDate(), 'd/m/Y');
}

Can we agree on that?

As an extension, the BlogController class shares this problem, it’s not OO and its state is created from within.

The problem:

-It’s impossible to configure the BlogController with different Views or Models because there is no real model or view. The state of the model and the view must always be supplied by the controller.

As I said, this is an important point and unless you can see that, it’s pointless to continue this debate

The knock on effect of this is that the application is stateless. You cannot pass around stateful view or model objects to other view or model objects. They are procedural and need to be supplied with their state every time they’re used. This goes back to my point about multiple instances of the same view with different states. When the view is just a template it needs to be supplied with its state every time which means means either repeated code or limited flexibility. This is identical to the difference between formatDate(‘d/m/Y’, $timestamp); and $date->format(‘d/m/Y’). $this->renderView(‘template.tpl’, $args); vs $view->render();

If you can’t understand or agree on that, I’m not sure I can continue this discussion. I’ve stripped it back to basics with the most trivial example possible.

Would you feel better if BlogController had some state?

class BlogController
{
    [COLOR="#FF0000"]public function __construct($doctrine, $templating)
    {
        $this->doctrine = $doctrine;
        $this->templating = $templating;
    }[/COLOR]

    public function listAction()
    {
        $posts = [COLOR="#FF0000"]$this->doctrine[/COLOR]->getManager()
            ->createQuery('SELECT p FROM AcmeBlogBundle:Post p')
            ->execute();

        return [COLOR="#FF0000"]$this->templating[/COLOR]->render(
            'AcmeBlogBundle:Blog:list.html.php',
            array('posts' => $posts)
        );
    }
}

Speaking of which… did you notice that your WireItUp code configures itself? The state of the model and the view is always supplied by your WireItUp code.

You seem to frequently forget that your WireItUp code is analogous to framework controllers, and every “criticism” that you’ve lobbed against framework controllers applies equally to your WireItUp code.

But that’s not the state, that’s simply using doctrine/templating in order to set it’s own state.

The state here the controller is “what template is being used?” along with “what data is needed?”

Whether it fires that state off to collaborators is irrelevant. You simply cannot use a different template in the listAction() method without repeating logic or restructuring the class after the fact. The state is always the same. Calling listAction() will always use the template ‘AcmeBlogBundle:Blog:list.html.php’ and always pass it the data from a query that runs: SELECT p FROM AcmeBlogBundle:Post p

It is still self-configuring. To fix this by making the smallest possible alteration to your existing code, you’d do this:


class BlogController
{
    public function __construct($doctrine, $templating, $template, $query)
    {
        $this->doctrine = $doctrine;
        $this->templating = $templating;
	$this->query = $query;
	$this->template = $template;
    }
    
    public function listAction()
    {
        $posts = $this->doctrine->getManager()
            ->createQuery($this->query)
            ->execute();

        return $this->templating->render(
            $this->template,
            array('posts' => $posts)
        );
    }
}

This is now a stateful object. Of course your models and views aren’t stateful but it’s a step in the right direction. Let’s make the model and view stateful as well:


class BlogController
{
    public function __construct($data, $template)
    {
	$this->data = $data;
        $this->template = $template;        
    }
    
    public function listAction()
    {
	return $this->template->render($this->data);
    }
}

This way, the controller could be constructed with any template and any data set and be reusable. It doesn’t Dig into collaborators and it doesn’t matter where the template comes from or where the data comes from. This creates a far more flexible application.

The template object could be anything, it doesn’t have to have loaded a .tpl.php file, it doesn’t have to generate HTML… it could generate a PDF, a CSV, a spreadsheet, etc and the data doesn’t have to have come from a database. It could have come from a CSV, a web service, anywhere.

All of which also applies to your WireItUp code.

You’re trying to fix problems that don’t exist, and you’re turning a blind eye to the fact that these “problems” also exist in your own code.

Once again, in any framework you need a router that does some decision making based on some kind of configuration. In your PAC framework, this is “This route initiates this controller and calls this action”. All of that “WireItUp” code can be configured here. “This route initiates this view with this template and passes it this model”.

You need a router that contains configuration in either scenario. Choosing a view is no different to choosing a controller in this sense. Your argument is flawed because you’re forgetting that you need some configuration somewhere. Your controller essentially duplicates this effort by having configuration spread across two different places.

If you honestly can’t see the advantages of:


class BlogController
{
    public function __construct($doctrine, $templating, $template, $query)
    {
        $this->doctrine = $doctrine;
        $this->templating = $templating;
    $this->query = $query;
    $this->template = $template;
    }
    
    public function listAction()
    {
        $posts = $this->doctrine->getManager()
            ->createQuery($this->query)
            ->execute();

        return $this->templating->render(
            $this->template,
            array('posts' => $posts)
        );
    }
}  

Over


class BlogController
{
    public function __construct($doctrine, $templating)
    {
        $this->doctrine = $doctrine;
        $this->templating = $templating;
    }
    
    public function listAction()
    {
        $posts = $this->doctrine->getManager()
            ->createQuery('SELECT p FROM AcmeBlogBundle:Post p')
            ->execute();

        return $this->templating->render(
            'AcmeBlogBundle:Blog:list.html.php',
            array('posts' => $posts)
        );
    }
}

Then I really can’t continue this discussion because you don’t understand the most basic OOP principles.