1
\$\begingroup\$

In this particular question, I need the reviewers help to judge my understanding to MVC and OOP as it was my biggest problem in my original question a month ago. I included in the question the contact page controller and the contact page template file


This post is a follow up for this and this.

What has been changed in general:

  1. I used front controller design pattern
  2. I left notepad++ and started to use eclipse IDE
  3. IStartedToFollowPSRStandard
  4. I started to turn to the magic of OOP
  5. I started to turn to the magic of MVC
  6. I separated the html in .tpl files
  7. I designed model classes to be used by the controllers
  8. I designed View class to be used by controllers to handle the .tpl files

The new project:

Of course my project didn't complete yet. but I built enough parts that I need your reviews on it, so I can change the wrong things before I go further with bad things.

I didn't want to go further in my project before you review this to tell me about my mistakes to avoid a lot of editing.


This is the current project files structure:

enter image description here

I separated the project into these parts:

  1. Views

    I made a folder with name "views" with sub directories called "templates", "js", "css" . I store the my html inside /templates/ folder with name .tpl which contains html code with variables. some variables are just native PHP variables like <?=$name?> and some other variables has special syntax like {{Mtpl@OPTIONS:[message-class]success-message|warning-message|error-message}}. I made this syntax to be parsed by View.class.php class that is used by the controllers to tell the controllers how to fill this variables. I also follow the "no CSS inside HTML" rule.

  2. Controllers

    The front controller will receive the request and decide 2 things:

    1. What is the action name intended by the request
    2. Which sub controller knows how to handle that action and then include it.(I'm planning to expand the front controller functionalities later to do things like logging the requests, blocking requests,.. etc).

    The controllers files have names like "/name.SE.php" SE is for "side effects" since PSR stated that a php file must either has definitions or do side effects. the controllers files has functions inside it with names exactly the same name as the action name that the is requested.

    Controllers job is to "fill templates variables" and echo the templates after filling it. controllers talk to the models throw the models classes public functions, and ask the models for things related to the data bases the controllers never knows anything about the databases.

  3. Models

    The model module is presented in my project as model classes that exposes public functions to be used by the controllers, functions like isLoggedIn(); isAccountingClient(); saveMessage(); . Only the models know how to connect to the data bases.

  4. Applications They're sub-applications like my accounting system, that I will put on the top of my website as a cloud services. each sub-application have the same structure (MVC) and a front controller-which is actually a sub controller of the primary domain front controller.

contact.SE.php

is the sub controller responsible for handling 2 actions (show contact page and send contact page form):

<?php
namespace aap;
// contact.SE.php (a controller designed to be included by the fron controller)
// uses: $sysModel
/*
 * @actions list:
 * 1-show
 * 2-send
 */
switch ($frontController->getAction()) {
    case "send":
        send($sysModel);
        break;
    default:
        show($sysModel);
        break;
}

function show(SystemModel $sysModel)
{
    $validator = new Validator($sysModel);
    $layout = new Layout("layout.tpl");
    $contact = new View("contact.tpl");

    $validator->validate($_GET['subject'], "firstOption", STRING);
    $subjectOptions = prepareSubjectOptions($_GET['subject']);
    $contact->set("subjectOptions", $subjectOptions);
    $layout->setProfile($sysModel);
    $layout->setApps($sysModel);
    $allViews = [
        $layout,
        $contact
    ];
    $layout->setJs($allViews);
    $layout->setCss($allViews);
    $layout->set("title", "Contact");
    $layout->set("contactSelected", "selected-button-s1");
    $layout->set("page", $contact->render());

    $finalOutput = $layout->render();
    echo $finalOutput;
}

function send(SystemModel $sysModel)
{
    $validator = new Validator($sysModel);
    $layout = new Layout("layout.tpl");
    $contact = new View("contact.tpl");
    //validating inputs
    $options[STRING]['length'] = 20;
    $validator->setOptions($options);
    $validator->validate($_POST['name'], "name", STRING, F_MUSTBESET );
    $options[STRING]['regex'] = "/[^0-9 +]/u";
    $options[STRING]['length'] = 15;
    $validator->setOptions($options);
    $validator->validate($_POST['phone'], "phone", STRING, F_MUSTBESET);
    $validator->resetOptions();
    $options[STRING]['regex'] = "/.*/u";
    $options[STRING]['ischarsinvalid'] = false;
    $options[STRING]['length'] = 500;
    $validator->setOptions($options);
    $validator->validate($_POST['message'], "message", STRING, F_REQUIRED | F_MUSTBESET | F_ESCAPE | F_SANITIZE);
    $validator->validate($_POST['email'], "email", EMAIL, F_MUSTBESET | F_ESCAPE | F_SANITIZE);
    $validator->resetOptions();
    $validator->validate($_POST['title'], "title", STRING, F_MUSTBESET | F_ESCAPE | F_SANITIZE);
    $validator->validate($_POST['subject'], "subject", STRING, F_REQUIRED | F_MUSTBESET | F_ESCAPE | F_SANITIZE);
    $errorMessage = "";
    foreach ($validator->getErrors() as $error => $value) {
        switch ($error) {
            case "name":
                $contact->set("nameInput-class", "incorrect-field");
                $errorMessage .= "<li>name field is not correct!</li>\n";
                // switch ($value['type']) {case ER_NOT_SET:
                break;
            case "phone":
                $contact->set("phoneInput-class", "incorrect-field");
                $errorMessage .= "<li>phone field is not correct!</li>\n";
                break;
            case "email":
                $contact->set("emailInput-class", "incorrect-field");
                $errorMessage .= "<li>email field is not correct!</li>\n";
                break;
            case "subject":
                $contact->set("subjectInput-class", "incorrect-field");
                $errorMessage .= "<li>invalid input!</li>\n";
                break;
            case "title":
                $contact->set("titleInput-class", "incorrect-field");
                $errorMessage .= "<li>title field is not correct!</li>\n";
                break;
            case "message":
                $contact->set("messageInput-class", "incorrect-field");
                $errorMessage .= "<li>message field is not correct!</li>\n";
                break;
        }
    }

    //setting the template vars

    if ($errorMessage) {
        $errorMessage = "<ul>$errorMessage</ul>";
        $contact->set("message", $errorMessage);
        $contact->set("message-class", "error-message");

        $contact->set("nameField", $_POST['name']);
        $contact->set("phoneField", $_POST['phone']);
        $contact->set("emailField", $_POST['email']);
        $contact->set("titleField", $_POST['title']);
        $contact->set("messageField", $_POST['message']);
    } else {
        $contact->set("message", "We succesiffuly recieved your message");
        $contact->set("message-class", "success-message");
        $contact->set("messageIcon", '<img class="icon" src="/media/correct.png">');
        $message = new Message($_POST['name'],$_POST['phone'], $_POST['email'],$_POST['subject'],$_POST['title'],$_POST['message']);
        $sysModel->saveMessage($message);
    }
    $subjectOptions = prepareSubjectOptions($_POST['subject']);
    $contact->set("subjectOptions", $subjectOptions);
    $layout->setProfile($sysModel);
    $layout->setApps($sysModel);
    $allViews = [
        $layout,
        $contact
    ];
    $layout->set("contactSelected", "selected-button-s1");
    $layout->setJs($allViews);
    $layout->setCss($allViews);
    $layout->set("title", "Contact");
    $layout->set("page", $contact->render());

    $finalOutput = $layout->render();
    echo $finalOutput;
}
// being called only inside this contact controller file.
function prepareSubjectOptions($firstSubject)
{
    $subjects = [
        'accounting question',
        'programming question',
        'complaint/complement',
        'I want to buy an application',
        'I want to try an application for free',
        'co-operation/partnership',
        'website feedback',
        'error/bug reporting',
        'other'
    ];

    if (in_array($firstSubject, $subjects)) {
        $orderedSubjects[] = $firstSubject;
        $orderedSubjects = array_merge($orderedSubjects, $subjects);
        $orderedSubjects = array_unique($orderedSubjects);
        $subjects = $orderedSubjects;
    }
    $subjectOptions = "";
    foreach ($subjects as $subject) {
        $subjectOptions .= "<option>$subject</option>\n";
    }
    return $subjectOptions;
}

contact.tpl

is the HTML template file. My templates contain 2 types of variables:

Native PHP variables like <?=$message?> and my template variables that has the pattern {{Mtpl@DIRECTIVE_NAME:DIRECTIVE_VALUE}}. The View class knows how to parse and deal with the templates and it is used by the controllers.

{{Mtpl@CSS:/views/css/general.css}}
<div id="contacttpl-body" class="body">
    <div id="message"
        class="{{Mtpl@OPTIONS:[message-class]success-message|warning-message|error-message}}">
        <?=$messageIcon?> <?=$message?></div>
    <img src='/media/smileface3.png'>
    <div id='contact'>
        <h2>Contact Me.</h2>
        <br> <br>
        <form method='post' action='/contact/send'>
            <table>
                <tr>
                    <td class='frmtext'>Name:</td>
                    <td class='inputs'><input class="{{Mtpl@OPTIONS:[nameInput-class]incorrect-field|}}" type='text' name='name'
                        value='<?=$nameField?>'></td>
                </tr>
                <tr>
                    <td class='frmtext'>Phone:</td>
                    <td class='inputs'><input class="{{Mtpl@OPTIONS:[phoneInput-class]incorrect-field|}}" type='text' name='phone'
                        value='<?=$phoneField?>'></td>
                </tr>
                <tr>
                    <td class='frmtext'>Email:</td>
                    <td class='inputs'><input class="{{Mtpl@OPTIONS:[emailInput-class]incorrect-field|}}" type='text' name='email'
                        value='<?=$emailField?>'></td>
                </tr>
                <tr height='10'>
                    <td></td>
                </tr>
                <tr>
                    <td class='frmtext'>Subject:</td>
                    <td class='inputs'><select name='subject'><?=$subjectOptions?>
                </select></td>
                </tr>
                <tr>
                    <td class='frmtext'>Title:</td>
                    <td class='inputs'><input class="{{Mtpl@OPTIONS:[titleInput-class]incorrect-field|}}" type='text' name='title'
                        value='<?=$titleField?>'></td>
                </tr>
                <tr>
                    <td class='frmtext'>Message: <span class="required">*</span></td>
                    <td class='inputs'><textarea maxlength='1000' name='message' class="{{Mtpl@OPTIONS:[messageInput-class]incorrect-field|}}"><?=$messageField?></textarea></td>
                </tr>
                <tr>
                    <td></td>
                    <td class='inputs'><input type='submit' value='submit'></td>
                </tr>
            </table>
        </form>
        <span class="small-size"><i><b>Phone, Email:</b> without any of them
                we will not be able to contact you back</i></span>
        <hr>
        <b>Or Send An Email to :</b><br>me[.@.]
        mynewdomain.com<br>
    </div>
</div>

These files has been omitted from the question to prevent the reviewers from being distracted. you can refer to them in the original post

  1. layout.tpl (a Template)
  2. View.class.php (a class that the controllers use to process the templates)
  3. Layout.class.php (a class the controllers use to process the layout Template)
  4. FrontController.class.php(a class the front controller uses)
  5. Validator.class.php (a class the controllers uses to validate the client input)
  6. SystemModel.class.php (an incomplete class the controllers use to talk to the model)
  7. .htaccess (an .htaccess file to apply the front controller)
  8. frontController.SE.php (is the front controller file)
\$\endgroup\$
  • 2
    \$\begingroup\$ You provide a bunch of background context which can be useful, though perhaps not to this extent. If you want the most honest code review, you should see if the code can begin to speak for itself. \$\endgroup\$ – Mike Brant Jan 6 '17 at 3:15
  • \$\begingroup\$ @MikeBrant . I'm afraid I didn't understand what you mean mike. do you mean that the new code doesn't seems to have the changes that I have listed ? like the MVC pattern and OOP ? . by the way thanks for http://www.phptherightway.com/ . I finished it's reading and it helped me alot \$\endgroup\$ – Accountant م Jan 6 '17 at 3:33
  • 2
    \$\begingroup\$ i meant that ideally good code should be pretty self explanatory. When you post a review such as this, it might be better to give only concise context as opposed to a really long explanation of the history of how you wrote the code. With good code, you shouldn't need this much explanation. Having an extremely lengthy question may also discourage reviewers. You might even consider breaking this up into smaller chunks for review, as there is a lot to consider here. I do think that you have made pretty remarkable improvement in a short time. \$\endgroup\$ – Mike Brant Jan 6 '17 at 5:07
  • 2
    \$\begingroup\$ Please change the titel, its very unclear what you are building based on the titel (thats what people see) \$\endgroup\$ – Gerrit Luimstra Jan 6 '17 at 7:23
  • \$\begingroup\$ @MikeBrant aha, Ok . I just thought that the details and information about the case might help the reviewers as the help center stated that "Problems with this question include:Lack of detail". the original post was even bigger but the moderators change it aggressively! . I originally posted this question to thank you Mike and thank @tim and @walther. you changed my php life thaaank you so much . but it seems the moderators don't like phrases like "thank you" in questions \$\endgroup\$ – Accountant م Jan 6 '17 at 12:25

Your Answer

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

Browse other questions tagged or ask your own question.