Is dit een goed MVC script?

Overzicht Reageren

Sponsored by: Vacatures door Monsterboard

Niels Vanderheyden

Niels Vanderheyden

03/10/2010 11:33:06
Quote Anchor link
Ik heb een mvc scriptje geschreven in php.
Nu vraag ik me af dit goed gescrheven is?

De controller:
Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
<?php
class LoginController{
    public $md5;
    private $getUsers;
    private $session;
    private $errors;
    private $captcha;
    private $loginModel;
    public $sendPage = "index.php";
    
    public function __construct($connection){
        require_once("NivaMVC/login/model/LoginModel.php");
        require_once("NivaMVC/login/view/LoginView.php");
        $this->loginModel = new LoginModel($connection);
        $this->errors = array();
        $this->captcha = false ;
        $this->md5 = true;
    }

    
    public function setCaptcha(){
        $this->captcha = true;    
    }

    
    public function validate(){
        $faults = false ;
        
        if(!(isset($_SESSION["nMVC_username"]) && isset($_SESSION["nMVC_password"]))){
            //Controle of er wel gepost is!
            if(isset($_POST["loginSend"])){
                //Kijkt na of je al juist bent ingelogd
                if(!$this->loginModel->checkLogin($_POST["username"],$_POST["password"])){
                    $this->errors[]["error"] = "Fout: de login is niet juist ingevuld!" ;
                    $faults = true;
                }

                
                if(!$faults){
                    //Er zijn geen fouten dus je wordt naar een andere pagina verzonden als je je nog op login bevind.
                    //$this->checkUrl("login.php",$this->sendPage);

                }
            }
        }

        return $faults;
    }

    
    public function view($template = "template1",$mode = "page"){
        $loginView = new LoginView($this->errors,$template,$this->captcha);
        if($mode == "page"){
            if((isset($_SESSION["nMVC_username"]) && isset($_SESSION["nMVC_password"]))) $loginView->loginComplete();
            else $loginView->loginForm();
        }

        else if($mode == "item"){
            if((isset($_SESSION["nMVC_username"]) && isset($_SESSION["nMVC_password"]))) $loginView->loginItemComplete();
            else $loginView->loginItem();    
        }
    }

    
    //Kijkt de naam van de file zit men nog op de login pagina dan wordt men naar de home pagina verzonden.
    private function checkUrl($url,$sendUrl){
        $loginNames = split("/",$_SERVER['SCRIPT_FILENAME']);
        $loginName = $loginNames[count($loginNames)-1] ;
        if($loginName == $url){
            $_SESSION["id"] = $this->loginModel->getid($_SESSION["username"],$_SESSION["password"]);
            header("Location:".$sendUrl);
        }
    }
    
}

?>


De model:
Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
<?php
    Class LoginModel{
        private $connection;
        
        public function __construct($connection){
            $this->connection = $connection;
        }

        
        //Hier wordt de login gecontroleerd
        public function checkLogin($username,$password){
            $password = md5($password);
            $login = false;
            $query = "SELECT  id,username,password  FROM users2 WHERE username = ? AND password = ? AND status = 'activate' ";
            if($stmt = $this->connection->prepare($query)){
                $stmt->bind_param("ss",$username,$password);
                $stmt->execute();    
                $stmt->bind_result($idRow,$usernameRow,$passwordRow);
                $rows = 0;
                while($stmt->fetch()){
                    $rows++;
                }

                if($rows > 0){
                    $login = true;
                    $_SESSION["nMVC_id"] = $idRow;
                    $_SESSION["nMVC_username"] = $username;
                    $_SESSION["nMVC_password"] = $password;
                }
            }

            return $login;
        }

        
        public function getid($username=false,$password=false){
            $query = "SELECT  username AS username,password AS password,id  FROM users2";
            $result = $this->connection->query($query);
            $userNames = array();
            $passwords = array();
            $ids = array();
            $users = array();
            $id = 0;
            while($row = $result->fetch_object()){
                if($row->username == $username && $row->password == $password){
                    $id = $row->id;    
                }
            }

            
            return $id;
        }
        
    }

?>


De view:
Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
<?php
class LoginView{
    public $errorMessage = "Fout: de login is niet juist ingevuld!" ;
    public $accountErrorMessage = "Fout: je account is nog niet geregistreerd!" ;
    private $model;
    private $errors;
    private $captcha;
    private $template;
    private $smarty;
    
    public function __construct($errors,$template,$captcha){
        $this->errors = $errors;
        $this->template = $template;
        $this->captcha = $captcha;
        // Haal Smarty binnen
        require_once("libs/smarty/Smarty.class.php");
        // Maak object
        $this->smarty = new Smarty();
    }

    
    public function loginForm(){
        // We kennen de variabelen toe
        $this->smarty->assign("action",$_SERVER["PHP_SELF"]);
        $this->smarty->assign("errors",$this->errors);
        $this->smarty->assign("postUsername",@$_POST["username"]);
        $this->smarty->assign("postPassword",@$_POST["password"]);
        $this->smarty->assign("captcha",@$this->captcha);
        $this->smarty->assign("uniceTime",@md5(uniqid(time())));
        // display it
        $this->smarty->display("login/loginForm/".$this->template.".tpl");
    }

    
    public function loginComplete(){
        // display it
        $this->smarty->display("login/loginComplete/".$this->template.".tpl");    
    }

    
    public function loginItem(){
        // display it
        $this->smarty->display("login/loginItem/".$this->template.".tpl");    
    }

    
    public function loginItemComplete(){
        // display it
        $this->smarty->assign("nMVC_username",$_SESSION["nMVC_username"]);
        $this->smarty->display("login/loginItemComplete/".$this->template.".tpl");    
    }
    
}

?>


Het aanroepen van het script:
Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
<?php require_once("nivaMVC/functions/include.php"); ?>

Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
<?php require_once("nivaMVC/login/controller/LoginController.php"); ?>

Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
4
5
<?php
    $loginController
= new LoginController($dbc);
    $loginController->validate();
    $loginController->view("template1");
?>


Graag jullie mening over dit loginsript.
De voordelen van dit script is ook dat je verschillende views hebt.
Je kan dus ipv het formulier de loginstatus weergeven!
Alvast bedankt.

Verplaatst naar OOP[/modedit]
Gewijzigd op 03/10/2010 13:28:31 door Chris -
 
PHP hulp

PHP hulp

24/11/2024 11:25:26
 
Gerard M

Gerard M

03/10/2010 11:45:57
Quote Anchor link
Als vuist regel kan je zeggen dat een model alleen maar getters en setters bevat. Jouw model bevat behoorlijk wat logica; en lijkt dus meer op bv. een database controller.
 
Niels Vanderheyden

Niels Vanderheyden

03/10/2010 12:10:24
Quote Anchor link
Dus mijn model aanpassen dat je dit kan benaderen via getters en setters?
 
Gerard M

Gerard M

03/10/2010 12:39:59
Quote Anchor link
Een model wordt alleen maar voor "storage" gebruikt. Een model voor een gebruiker kan er bv. zo uitzien:

Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
<?php
class UserModel {
    private $username;
    private $userid;
    private $password;
    
    public function __construct($username, $userid, $password) {
        $this->username = $username;
        $this->userid   = $userid;
        $this->password = $password;
    }

    public function getUsername() {
        return $this->username;
    }

    public function setUsername($value) {
        $this->username = $value;
    }

    public function getUserid() {
        return $this->userid;
    }

    public function setUserid($value) {
        $this->userid = $value;
    }

    public function getPassword() {
        return $this->password;
    }

    public function setPassword($value) {
        $this->password = $value;
    }

    
    public function __toString() {
        return "[[UserModel] {$this->username} ] ";
    }
}

?>


Je controller zal dit UserModel object aanmaken en je view kan deze dan weer benaderen via de controller. De model zelf heeft geen logica, en bevat alleen maar "domme" getters/setters.

Daarnaast is het handig als je functie namen uit een werkwoord + zelfstandig naamwoord bestaan. Dus "validateLogin" ipv "validate". Daarnaast is "setCaptcha" onduidelijk (je verwacht dat je een parameter mee kan geven) en had "enableCaptcha" een mooier alternatief kunnen zijn.

Toevoeging op 03/10/2010 13:04:41:

Tevens nog een aantal andere puntjes waar je rekening mee kan houden;

Het commentaar in jouw code voegt niets toe. Zo zie ik bijvoorbeeld:
" // Maak object
$this->smarty = new Smarty();"

Het idee van commentaar is te vertellen waarom je iets doet, en niet wat je doet. Waarom maak ik een Smarty object? Waar wordt deze voor gebruikt? Wat is Smarty uberhaubt?

Daarnaast vindt ik je variable naamgeving onduidelijk. Bij je controller zie ik ineens "public $captcha;" later na het bestuderen van de code vind ik uit dat het een boolean is. Had "$isCaptchaEnabled" niet een betere keuze kunnen zijn? Tevens weer een mooie vuistregel; Laat je boolean variables met "is" beginnen. Daarnaast vind ik de variable met de naam "$getUsers;" ook erg onduidelijk; hij heeft de naam van een functie!

Wat mij ook opvalt is dat je zomaar "$_SESSION" gebruikt, zonder te kijken of session_start() uberhaubt gebruikt is.
Gewijzigd op 03/10/2010 12:40:41 door Gerard M
 
Jelmer -

Jelmer -

03/10/2010 13:23:16
Quote Anchor link
Gerard M op 03/10/2010 11:45:57:
Als vuist regel kan je zeggen dat een model alleen maar getters en setters bevat. Jouw model bevat behoorlijk wat logica; en lijkt dus meer op bv. een database controller.


Logica in je model lijkt mij prima, zolang het maar gaat om logics die alleen maar slaat op de data in je applicatie (en het beheer daarvan) en niet de werking van deze specifieke applicatie. Het idee is dat het model los staat van de rest van de applicatie, en op zichzelf een interface is tot alle data. Op die interface kan je dan verschillende applicaties bouwen, zoals nu een website, maar in principe ook een desktop app of een webservice. Als al die dingen hetzelfde model gebruiken, kan je je model voor de verantwoordelijkheid van je data op laten draaien. Als je model goed is, kan geen van al die applicaties je data misbruiken of ongeldig maken. Je schermt al die verantwoordelijkheid af. En dat doet hij volgens mij prima op deze manier.

Over hoe praktisch en netjes je implementatie is valt nog wat te twisten, maar volgens mij is dit een prima implementatie van het idee MVC. Je controller stuurt de boel aan, je view is volledig verantwoordelijk voor de weergave, en je model is volledig verantwoordelijk voor de data. Welk boek of tutorial heb je gebruikt?
Gewijzigd op 03/10/2010 13:24:05 door Jelmer -
 
Niek s

niek s

03/10/2010 14:20:35
Quote Anchor link
MVC staat niet vast he. Het is een idee, en relatief "vaag" omschreven. Er zijn geen exacte regels voor.
Daarom kan je nooit exact "volgens het MVC principe" programmeren. MVC is meer een opvatting, en zal van persoon op persoon verschillen.
Natuurlijk kun je wel zeggen "dit is MVC geschreven", of "volgens het MVC principe". Maar fout en goed is lastig aan te geven op dit gebied. Dit zal ook verschillen per applicatie. Het is bedoeld om je te helpen, en niet om je in de weg te zitten omdat iets lastiger blijkt te zijn als je het volgens MVC in elkaar schuift.

Een Model is, zoals Jelmer al zei, niet alleen "storage". Logica is prima in een Model, echter zit er een verschil in logica. Wat versta jij onder logica?
Een Model is in principe bedoeld om de data op te slaan. Meestal gebeurd dit dus op 2 niveau's: View en Model (al dan niet Database).
Over het algemeen doet de Controller het "echte" werk. De Controller bepaald alleen (!) welke acties gedaan worden.
De view wordt echter meestal niet "direct" aangestuurt, hierdoor krijg je dubbele logica: je schrijft namelijk data weg in je view en in je Model. Om dus verkeerde informatie op je scherm uit de weg te gaan, gebruik je een zogenoemde "Observer" (Google). Dit houd in principe in dat je je View update aan de hand van de data die in je Model staat. Simpelweg door een update aan te roepen, zal 'iets' het Model uitlezen en de view aanpassen.

Door mijn persoonlijke ervaring vind ik bovenstaande het prettigste werken, echter kan hier natuurlijk vanaf geweken worden per applicatie.

Nog opmerkingen over deze specifieke applicatie:
- Regel 3 en 4 van je View. Denk eens na over het feit of die zinnen in de applicatie zelf aangepast zullen worden. Oftewel: gaat jouw applicatie die specifieke zin aanpassen voordat deze getoond wordt? Waarschijnlijk niet. In dat geval kun je deze dus beter een constante maken, hij wordt toch niet aangepast. Maar waarom heb je ze public gemaakt?
Gewijzigd op 03/10/2010 14:22:28 door niek s
 
Niels Vanderheyden

Niels Vanderheyden

03/10/2010 15:20:37
Quote Anchor link
Alvast bedankt voor de goede feedback en tips.

Jelmer vanwaar ik het mvc heb gehaald is, is nog moelijk te zeggen.
Maar hoofdzakelijk heb ik mvc geleerd op school nl in as3 en verder van enkele tutorials over php en dan verder gegaan volgens mijn denkwijze.
 



Overzicht Reageren

 
 

Om de gebruiksvriendelijkheid van onze website en diensten te optimaliseren maken wij gebruik van cookies. Deze cookies gebruiken wij voor functionaliteiten, analytische gegevens en marketing doeleinden. U vindt meer informatie in onze privacy statement.