4
\$\begingroup\$

I am working on creating a simple MVC inspired framework (for learning purposes and because i don't want to use a pre-existing framework at the moment). I have the system already set up and its working as it should, but i would like to have some feedback on the "view" part of the system, maybe i have set it up in a way that will prove to be inefficient in the future if the app grows, and maybe there is just a better way of doing what i am doing.

A little bit about how i set up the "framework":

When a user goes to a url, for example /posts/post_id, the file that is loaded is index.php, this file instantiates a router file, the router instantiates the required controller (and method if set) based on the url. The controller works with the relevant model to get data from the database (in this example, the post data), and then the controller sends that data off to the view class. The controller also sends the "view settings" to the view class.

What i mean by view settings and my use of it and view data:

The way i have set up the "View" part of this system is to be able to render either an entire page, which includes the template (header, footer, sidebar, etc.), or to render just a single html file (without the header, footer, etc.).

To do this, i send from the controller to the view class the following settings:

  1. The view settings - what navigation menu to show, if to show a sidebar and its location on the page (left, right, none), if to render just a file with html without the whole template (e.g. just a post, or a form), or a whole page with the template.
  2. The view data - the actual data needed for the view, for example the post data.
class View{

    /*  $viewData is sent from the controller, it contains all of the data needed
    *   for the view, including what i call the "view settings" as described above */
    public function __construct($viewData){

        //Instantiate a new error for this view (error handling class)
        $this->err = new ErrorClass(get_class($this));

        //Instantiate a new helper (helper functions used in the view)
        $this->helpers = new ViewHelpers();

        /* Save variables from the $viewData array */

        //If a single file was sent to render (receives the file name and is rendered later)
        $this->viewFile     = $viewData['view_file'];

        //If a whole page was sent to be rendered (this will render the whole template also)
        $this->viewName     = $viewData['view_name'];

        //Template data, only used if a whole page should be rendered
        $this->templateData = $viewData['template_data'];

        //The data for the view, e.g. post data
        $this->viewData     = $viewData['view_data'];

        //Call the render view method
        $this->renderView();
    }



    /* Function to render the view output
    ===============================================*/
    private function renderView(){

        //If a single file should be rendered
        if($this->viewFile && file_exists(VIEWS_PATH.$this->viewFile)){
            require(VIEWS_PATH.$this->viewFile);


        //If a whole page should be rendered
        }elseif($this->viewName && file_exists(TEMPLATE_FILE_PATH)){
            require(TEMPLATE_FILE_PATH);


        //Save an error
        }else{
            echo 'Oops, something went wrong, we are looking into it!';
            $this->err->setError(array('err_cat' => 3, 'err_msg' => "View wasn't able to load"));
        }
    }
}

A note about VIEWS_PATH and TEMPLATE_FILE_PATH:

VIEWS_PATH is the path to the folder that contains all of the html files, when i use require(VIEWS_PATH.$this->viewFile); i am including the file name that was sent from the controller, so this line would be: require('/app/views/post_form.php'); for example.

TEMPLATE_FILE_PATH is a constant that equals the path to a main template file, what i mean by this is TEMPLATE_FILE_PATH = /public/template/template.php, and this file includes in it the header, the footer, and the actual main page that is needed, for example if loading a post form, the page would include the header, then the post form file, and then the footer.

EDIT: I understand that my initial question was not very clear, so i edited it, hopefully this is clearer.

\$\endgroup\$
  • 1
    \$\begingroup\$ Hello again, by coincidence. Unfortunately it's not clear what you understand by view and by template. This phrase is confusing: " load either a single file, or an actual view through including the template". Also, in the 1st elseif statement, how is $this->viewName related to TEMPLATE_FILE_PATH? Maybe some value examples for the variables will make the whole picture clearer. And the constructor, it would be of much help to post it too. \$\endgroup\$ – dakis May 25 at 22:50
  • \$\begingroup\$ A view should be a class, like UI\Web\View\Users\AddUser or UI\Web\View\Users\ListUsers. This class should be able to load and render a certain template file, like <project-root>/web/src/templates/templates/Users/add-user.html.twig or <project-root>/web/src/templates/templates/Users/list-users.html.twig. So, a specific controller would call the render() method of a specific view instance, which will load and render a specific template file. To remember is, that a view should not be confused with a template file (or, short, template). \$\endgroup\$ – dakis May 25 at 23:02
  • \$\begingroup\$ P.S: The .twig extension denotes the fact that a template engine (like Twig) can be used, in order to render the template files. In such a case, an instance of it would be injected into the specific view object. \$\endgroup\$ – dakis May 25 at 23:16
  • 3
    \$\begingroup\$ @dakis Please only seek clarification via comments. Please post "review" advice as an answer. Future researchers will not expect to see bits of review in comments under the question. Please adhere to the simple/clear design of this site when posting. \$\endgroup\$ – mickmackusa May 26 at 0:43
  • 1
    \$\begingroup\$ @mickmackusa I will, thanks for the advice. Tried to give K.D a direction for her/his eventual question edits. Then I will transfer my previous two comments to an eventual answer, or remove them. \$\endgroup\$ – dakis May 26 at 7:28
2
\$\begingroup\$

To be honest, I am not sure whether this question is on topic. There is literally only a handful lines of code, just a single conditional operator. With so much (as architecture, intentions, the desired goal) left out. But well.

I have but two suggestions.

  1. Use Twig. It will solve ALL your problems at once.
    • Twig supports template inheritance. This is an enormously helpful feature, in your case it will let a particular template to decide whether to include the main site template or not. As a result, no need to mingle with viewName, viewFile, viewAnything. Just call a template, that's all.
    • The main template will be a single file instead of a bunch of separate "header, footer, sidebar... whatever" files, which helps editing a lot.
    • Twig supports blocks as well, so again you can move your sidebar around in every other template.
  2. Error reporting. The current approach is flawed. In how many places you are going to add that echo 'Oops, something went wrong, we are looking into it!';? and what if you some day will decide to add some HTML to it? Or what if a client will expect JSON-formatted error, not a plain text? Instead of that clumsy echo with an error class combination you should really use an error/exception handler. In case a file is not found, an error will be raised, then your handler will intercept it, and then, in the handler, you are free to echo any text and use whatever logging method which pleases you. All in a single place. As a result, you will have
    • less code to write
    • centralized error processing
    • the more informative error message (the filesystem will tell you the exact reason instead of your generic "View wasn't able to load").

for more information see my article on the principles of PHP error reporting.

\$\endgroup\$
  • \$\begingroup\$ Thanks for the input! About the error reporting - i do actually have an error reporting class that i wrote, it may not be perfect as i am still learning this and working on it, but it is pretty dynamic and for the moment, set up in a way that works as i need it to, but of course can be improved (a lot!). The echo in the code i wrote is not part of the error class, that echo is more of a temporary thing to show that the requested file was not found, so that is something that will change. \$\endgroup\$ – K.D May 28 at 14:06
  • \$\begingroup\$ As for using Twig, at this point i am trying to learn all i can about creating this whole type of system from scratch, not using any engines, frameworks, or anything else. So i would much prefer to understand the logic behind how it works and try to write my own code for it, rather than using something already made. From what i understand you say that Twig is set up in a way that the template file knows what to include? Does that mean that this file receives a variable or some sort of data, indicating what it should show on the page? \$\endgroup\$ – K.D May 28 at 14:09
  • \$\begingroup\$ it is simply written in the template itself. twig.symfony.com/doc/2.x/tags/extends.html \$\endgroup\$ – Your Common Sense May 28 at 14:19
  • \$\begingroup\$ And if i don't want to include a whole page that includes header, footer, etc. and just include a file containing a form, or some other html, it would just be a matter of including that file rather than the full page template file no? \$\endgroup\$ – K.D May 28 at 14:32
  • \$\begingroup\$ you just dont add extends keyword \$\endgroup\$ – Your Common Sense May 28 at 14:42

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.