7
\$\begingroup\$

I know the Model, Controller and View's purpose, but I have never really found a good concrete class example of a View class.

Usually I see people having some small render() method being called from the Controller, or their Controller holds all the View logic. So I tried my best to create a View class that seemed proper and somewhat clean to me in regards to the MVC pattern.

I've found a useful image that shows how the data flow is supposed to be and how the View sits in it.

  • Is this a valid View class?

  • Anything I could improve on / suggestions?

    Readability, usability and / or efficiency.


LoginView.php

namespace View;

use View\View;

class LoginView extends View
{
    private $isUserLoggedIn;
    private $isFormTokenValid;

    public function index()
    {
        // Logged in Users have no reason to view this page.
        if ($this->isUserLoggedIn) {
            $this->httpResponse->redirect('/');
        }

        // Assume csrf attack, refresh self for a fresh View.
        if ($this->isFormTokenValid === false) {
            $this->httpResponse->redirect();
        }

        // Header title.
        $this->templateData['title'] = 'Log In - Site Name';

        // Header stylesheets.
        $this->templateData['styleSheets'] = [
            '/stylesheets/login.css'
        ];

        // Header javascripts.
        $this->templateData['javaScripts'] = [
            '//ajax.googleapis.com/ajax/libs/jquery/1.11.1/jquery.min.js',
            '/javascripts/login-form-handler.min.js'
        ];

        // Form input error messages.
        $formErrorMessages = [
            'email' => [
                'NotEmpty' => 'Enter your email address.',
                'Email'    => 'You did not enter a valid email address.'
            ],
            'password' => [
                'NotEmpty' => 'Enter your password.'
            ]
        ];

        // Replace form error codes with corresponding error messages.
        if (isset($this->templateData['form']['errors'])) {
            $this->templateData['form']['errors'] = $this->replaceArrayValuesByKey($this->templateData['form']['errors'], $formErrorMessages);
        }

        // If not set by User input, check 'remember' checkbox by default.
        if (!isset($this->templateData['form']['values']['remember'])) {
            $this->templateData['form']['values']['remember'] = true;
        }

        $this->renderTemplate();
    }

    public function setIsUserLoggedIn($boolean)
    {
        $this->isUserLoggedIn = $boolean;
    }

    public function setIsFormTokenValid($boolean)
    {
        $this->isFormTokenValid = $boolean;
    }

    public function setFormValueOf($name, $value)
    {
        $this->templateData['form']['values'][$name] = $value;
    }

    public function setFormErrorCodeOf($name, $code)
    {
        $this->templateData['form']['errors'][$name] = $code;
    }

    public function setHasLoginFailed($boolean)
    {
        $this->templateData['hasLoginFailed'] = $boolean;
    }
}

View.php

namespace View;

use Http\HttpResponse;

abstract class View
{
    protected $httpResponse;
    protected $templatePaths;
    protected $templateData;

    public function __construct(
        HttpResponse $httpResponse,
        $headerTemplatePath = null,
        $bodyTemplatePath   = null,
        $footerTemplatePath = null
    ) {
        $this->httpResponse = $httpResponse;

        if ($headerTemplatePath) {
            $this->templatePaths['header'] = $headerTemplatePath;
        }

        if ($bodyTemplatePath) {
            $this->templatePaths['body'] = $bodyTemplatePath;
        }

        if ($footerTemplatePath) {
            $this->templatePaths['footer'] = $footerTemplatePath;
        }
    }

    public function replaceArrayValuesByKey(array $target, array $source)
    {
        foreach ($target as $k => $v) {
           for ($i = 0, $c = count($v); $i < $c; ++$i) {
              if (isset($source[$k][$v[$i]])) {
                   $target[$k][$i] = $source[$k][$v[$i]];
              }
           }
        }

        return $target;
    }

    public function renderTemplate()
    {
        extract($this->templateData);

        if (isset($this->templatePaths['header'])) {
            require_once $this->templatePaths['header'];
        }

        if (isset($this->templatePaths['body'])) {
            ob_start([$this, 'autoIndent']);

            require_once $this->templatePaths['body'];

            ob_end_flush();
        }

        if (isset($this->templatePaths['footer'])) {
            require_once $this->templatePaths['footer'];
        }
    }

    private function autoIndent($buffer)
    {
        $content = '';
        $lines   = explode(PHP_EOL, $buffer);

        // TODO: calculate indentation levels of header template

        foreach ($lines as $line) {
            $content .= str_repeat(' ', 12) . $line . PHP_EOL;
        }

        return $content;
    }
}

You can view the LoginController class that controls this View here, if interested.

\$\endgroup\$
2
  • 1
    \$\begingroup\$ A view is presentation of data. A HTTPResponse thus can have a View as it's content. Not the other way round. You are calling redirect in a view. This is bad bad bad! \$\endgroup\$
    – Pinoniq
    Sep 12, 2014 at 15:20
  • \$\begingroup\$ @Pinoniq You're correct, a View is a presentation of data to the User (which is the browser). Redirection is setting the HTTP Location: header, thus it is the View's responsibility. The model layer does not know the location of pages and neither does the controller. Only the View. \$\endgroup\$ Sep 15, 2014 at 9:10

1 Answer 1

2
\$\begingroup\$

As @Pinoniq mentioned in the comments, a view shouldn't be responsible for redirect users via business rules. The responsibility of a view should be solely to convert a PHP template into HTML and to capture that output into a variable to be used as you see fit.

Based on that criteria alone, the redirect violates that rule. That sort of stuff should all be happening at the controller level. As soon as you require data to be passed into a view, your controller will need to check whether the user is logged in or not before it can fetch their corresponding user data so it makes sense to have it there from that perspective alone.

Not to mention from a testing perspective, it's much easier to test that re-direct at the controller level than it is to test it in the view. This also removes the dependency on HttpResponse which means that you don't need to mock a response just to test your view creation logic now which is also a big win. Assuming you're writing tests for all of this logic ;)

Finally, I'd highly recommend taking a look at Twig or some other PHP compatible template engine. It seems to me like you're getting a bit over zealous in defining a lot of your page header and body blocks programatically whereas you'd be much better off in taking this same approach, but with template inheritance instead, in a template engine.

P.S. I don't think initializing an array with = [ ]; is valid PHP for your $formErrorMsg.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.