2
\$\begingroup\$

It's not finished yet, but I want to know if the structure and the classes are ok and what can I change. Feel free to say anything.

To be more specific, I want to know what you think about the login function in the Login_Controller class:

public function login() {
            if (!empty($_POST['username']) && !empty($_POST['password'])) {
                    $user = new Users_Model;

                    $username = $_POST['username'];
                    $password = $_POST['password'];

                    if ($iduser = $user->is_registered($username, $password)) {
                            $_SESSION['user'] = $username;
                            $_SESSION['iduser'] = $iduser;
                            if ($_POST['keep']) {
                                    $user->save_cookie($username);
                            }
                            header('Location: ' . SITE_ROOT);
                            exit();
                    } else {
                            $error = 'El usuario o el password no son correctos.';
                            $this->main($error);
                    }
            } else {
                    $error = 'No ingreso el usuario o el password.';
                    $this->main($error);
            }
    }

And the router as well:

class Router {
    static function init($request)
    {
            $parsed = explode('/', $request);
            if ($parsed[0] == 'admin') {
                    $page = array_shift($parsed).ucfirst(array_shift($parsed));
            } else {
                    $page = array_shift($parsed);
            }
            if (empty($page)) {
                    $page = 'index';
            }
            $action = array_shift($parsed);
            if (empty($action)) {
                    $action = 'main';
            }
            if (empty($parsed)) {
                    $arguments = 0;
            } else {
                    $arguments = $parsed;
            }
            $target = SERVER_ROOT . '/controllers/' . $page . '.php';
            if (file_exists($target)) {
                    include_once $target;
                    $class = ucfirst($page) . '_Controller';
                    if (class_exists($class)) {
                            $controller = new $class;
                    } else {
                            die('la clase no existe');
                    }
            } else {
                    die('la pagina no existe');
            }
            $controller->$action($arguments);
    }
}
\$\endgroup\$
1
  • \$\begingroup\$ I don't think you get the point of MVC :( \$\endgroup\$
    – Pinoniq
    Oct 22 '13 at 10:17
1
\$\begingroup\$

They are ok, but I would still do some things differently.

  1. I would instead throw exceptions.
  2. Instead of if else within your action, I would use a validator with rules.
  3. Instead of using $_POST/$_GET directly, I would sanitize them and put it inside a Request class.
  4. Error messages belongs to the view, so I would set $view->errors[]='message' or something like that.
  5. Your routing class is not flexible. As a good example, you could take a look at the Silex framework.
  6. Login_Controller should not exist. There might be Account_Controller with login_action or Authenticate_Controller with login_action.

Usually a Controller is a group of actions with to execute specific stuffs. For example, Profile_Controller has view/edit/delete action. Within the controller you check the user roles, if someone can view/edit/delete the profile. In other frameworks, they use "Access Control List (ACL)" for this purpose.

Your linked tutorial is just for the basic understanding purpose, so please don't use it. Use a readily existing framework, since you wish to build the app and not the framework itself. You might end with modifying and changing your basic structures and implementing new functions instead of developing the features of your web application.

\$\endgroup\$
2
  • \$\begingroup\$ Thanks for the answer. Right now I'm taking a look to the Fabien Potencier tutorial for a framework, after that I will take a look to Silex. \$\endgroup\$ Oct 23 '13 at 2:28
  • \$\begingroup\$ also please note that a Framework is just a delivery detail, it is not really required, with Clean Code Architecture which was introduced by Robert Ceciel Martin (aka. Uncle Bob) youre able to create software and switch the frameworks with less efford \$\endgroup\$
    – BlackScorp
    Oct 23 '13 at 6:23

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.