Goed op weg?

Overzicht Reageren

Sponsored by: Vacatures door Monsterboard

Roeltje M

Roeltje M

23/06/2010 22:53:54
Quote Anchor link
HOi,

Ik ben een redelijke beginner op het gebied van OOP en vroeg mij af of ik goed op weg was met:

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
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
<?php
class InvalidLoginException extends Exception { };
class InvalidNewsMessageException extends Exception { };
class InvalidCategoryException extends Exception { };
class InvalidOutbreakException extends Exception { };
class InvalidImageException extends Exception { };
class InvalidRssException extends Exception { };

class UserSessionController
{

    protected $db;    
        
    function __construct(PDO $db)
    {
        $this->db = $db;
    }
    
    private function checkUsername($user)
    {
        if(strlen($user) < 3)
            throw new InvalidLoginException('Gebruikersnaam te kort.');    
            
        return true;            
    }
    
    private function checkPassword($pass)
    {
        if(strlen($pass) < 6)
            throw new InvalidLoginException('Wachtwoord te kort.');            
    
        return true;            
    }    
    
    private function checkCombination($user,$pass)
    {
        $query = "SELECT user_id, user_loginname, user_level FROM dxtr_user WHERE user_loginname = :user AND user_password = :pass";
        $stmt = $this->db->prepare($query);    
        $stmt->bindParam(':user', $user);
        $stmt->bindParam(':pass', md5($pass));
        $stmt->execute();
        
        if($stmt->rowCount() != 1)
            throw new InvalidLoginException('Combinatie gebruiker/wachtwoord onbekend');
        
        return $this->fetch($stmt);
        
    }
    
    protected function fetch(PDOStatement $stmt)
    {
        $data = $stmt->fetch(PDO::FETCH_ASSOC);
        
        return $data;
    }    
    
    protected function fetchAll(PDOStatement $stmt)
    {
        $data = $stmt->fetchAll(PDO::FETCH_ASSOC);
        
        return $data;
    }        
    
    public function login($username,$password)
    {
        $this->checkUsername($username);
        $this->checkPassword($password);
            
        if($user = $this->checkCombination($username,$password))
        {
            $_SESSION['USER']['ID'] = $user['user_id'];
            $_SESSION['USER']['NAME'] = $user['user_loginname'];    
            $_SESSION['USER']['LEVEL'] = $user['user_level'];                
            $_SESSION['USER']['LOGGEDIN'] = true;        
        }
        
        return true;
    }
}

class NewsController
{
    protected $db;    
        
    function __construct(PDO $db)
    {
        $this->db = $db;
    }
    
    public function getNews()
    {
        $query = "SELECT n.news_id, n.news_title, DATE_FORMAT(n.datum, '%d-%m-%Y') as datum FROM dxtr_news n ORDER BY n.datum DESC, n.news_id DESC";
        $stmt = $this->db->prepare($query);            
        $stmt->execute();        
        
        return $this->fetchAll($stmt);
    }    
    
    public function getNewsPictureById($id)
    {
        $query = "SELECT nie_id, afbeelding FROM nieuws_afbeelding WHERE nie_id = :id";
        $stmt = $this->db->prepare($query);    
        $stmt->bindParam(':id', $id);        
        $stmt->execute();        
        
        if($stmt->rowCount() != 1)
            throw new InvalidNewsMessageException ('Geen afbeelding gevonden');        
        
        return $this->fetch($stmt);        
    }    
    
    public function getHomePageNews($van,$tot)
    {
        $query = "SELECT news_id, news_title, DATE_FORMAT(FROM_UNIXTIME(news_datestamp), '%d %M %Y') as news_date, news_thumbnail, news_body, news_extended FROM dxtr_news n ORDER BY news_date DESC, news_id DESC LIMIT ".$van.",".$tot;
        $stmt = $this->db->prepare($query);            
        $stmt->execute();                
        
        return $this->fetchAll($stmt);
    }        
    
    
    public function getNewsById($id)
    {
        $query = "SELECT n.nie_id, nc.ncat_id, n.bericht_afbeelding, n.titel, n.bericht, DATE_FORMAT(FROM_UNIXTIME(news_datestamp), '%d-%m-%Y') as news_date, nc.naam FROM nieuws n LEFT JOIN nieuws_categorie nc ON (nc.ncat_id = n.ncat_id) WHERE n.nie_id = :id";
        $stmt = $this->db->prepare($query);    
        $stmt->bindParam(':id', $id);        
        $stmt->execute();        
        
        if($stmt->rowCount() != 1)
            throw new InvalidNewsMessageException ('Nieuwsbericht niet gevonden');        
        
        return $this->fetch($stmt);        
    }
    
    public function getCategories()
    {
        $query = "SELECT ncat_id, naam FROM nieuws_categorie ORDER BY ncat_id ASC";
        $stmt = $this->db->prepare($query);            
        $stmt->execute();        
        
        if($stmt->rowCount() < 1)
            throw new InvalidCategoryException ('Er is nog geen categorie toegevoegd');                
        
        return $this->fetchAll($stmt);
    }
    
    public function getNewsCount()
    {
        $query = "SELECT COUNT(*) as aantal FROM nieuws";
        $stmt = $this->db->prepare($query);            
        $stmt->execute();    
        
        return $this->fetch($stmt);                
    }
    
    public function setNews($titel,$bericht,$ext_bericht,$datestamp,$id,$samenvatting,$img,$sticky)
    {
        if(strlen($titel) < 6)
            throw new InvalidNewsMessageException ('Nieuwstitel moet 6 tekens of meer bevatten');
        if(strlen($bericht) < 20)
            throw new InvalidNewsMessageException ('Nieuwsbericht moet 20 tekens of meer bevatten ('.strlen($bericht).')');    
        if(empty($img["name"]))
            throw new InvalidNewsMessageException ('Er is geen thumbnail geselecteerd');                

        $query = "INSERT INTO dxtr_news (news_id,news_title,news_body,news_extended,news_datestamp,news_author,news_summary,news_thumbnail,news_sticky) VALUES (DEFAULT,:titel,:bericht,:ext_bericht,:datum,:id,:samenvatting,:pic,:sticky)";        
        $stmt = $this->db->prepare($query);    
        $stmt->bindParam(':titel', $titel);    
        $stmt->bindParam(':bericht', $bericht);    
        $stmt->bindParam(':ext_bericht', $ext_bericht);    
        $stmt->bindParam(':datum', $datestamp);    
        $stmt->bindParam(':id', $id);    
        $stmt->bindParam(':samenvatting', $samenvatting);                        
        $stmt->bindParam(':sticky', $sticky);                        
        if($this->checkImage($img,NULL,NULL))
        {
            $imgname = "images/news/".$this->storeImage($img);
        }                
        $stmt->bindParam(':pic', $imgname);        

        $stmt->execute();                
    }
    
    public function setNewsPictureById($id, array $img)
    {
        if(empty($img["name"]))
            throw new InvalidImageException ('Er is geen afbeelding geselecteerd.');            

        if($this->checkImage($img,436,200))
        {
            $imgname = "http://www.epidemie.nl/uploads/".$this->storeImage($img);
        }    
        
        $query = "INSERT INTO nieuws_afbeelding (nie_id,afbeelding) VALUES (:id,:img)";        
        $stmt = $this->db->prepare($query);    
        $stmt->bindParam(':id', $id);                
        $stmt->bindParam(':img', $imgname);                                                                        
        $stmt->execute();            
    }
    
    public function setNewsByUpdate($titel,$pic,$cat,$bericht,$id)
    {
        if(strlen($titel) < 6)
            throw new InvalidCategoryException ('Nieuwstitel moet 6 tekens of meer bevatten');
        if(strlen($bericht) < 20)
            throw new InvalidCategoryException ('Nieuwsbericht moet 20 tekens of meer bevatten');            
            
        if(empty($pic["name"]))
        {
            $query = "UPDATE nieuws SET ncat_id=:cat_id, titel=:titel, bericht=:bericht WHERE nie_id = :id";    
        }
        else
        {
            if($this->checkImage($pic,NULL,NULL))
            {
                $imgname = "images/news/".$this->storeImage($pic);
            }                
            $query = "UPDATE nieuws SET ncat_id=:cat_id, titel=:titel, bericht_afbeelding=:pic, bericht=:bericht WHERE nie_id = :id";                
        }            

        $stmt = $this->db->prepare($query);    
        if(!empty($pic["name"])) {             $stmt->bindParam(':pic', $imgname);        }
        $stmt->bindParam(':cat_id', $cat);        
        $stmt->bindParam(':id', $id);                
        $stmt->bindParam(':titel', $titel);                
        $stmt->bindParam(':bericht', $bericht);                                            
        $stmt->execute();                                
    }    
    
    public function setCat($naam)
    {
        if(strlen($naam) < 5)
            throw new InvalidNewsMessageException ('Nieuwstitel moet 5 tekens of meer bevatten');

        $query = "INSERT INTO nieuws_categorie (ncat_id,naam) VALUES (DEFAULT,:naam)";        
        $stmt = $this->db->prepare($query);    
        $stmt->bindParam(':naam', $naam);                                        
        $stmt->execute();                
    }    
    
    public function deleteNewsById($id)    
    {
        $query = "DELETE FROM nieuws WHERE nie_id = :id";
        $stmt = $this->db->prepare($query);            
        $stmt->bindParam(':id', $id);                                
        $stmt->execute();                
    }
    
    public function deleteNewsPictureById($id)    
    {
        $query = "DELETE FROM nieuws_afbeelding WHERE nie_id = :id";
        $stmt = $this->db->prepare($query);            
        $stmt->bindParam(':id', $id);                                
        $stmt->execute();                
    }
    
    public function deleteCatById($id)    
    {
        $query = "DELETE FROM nieuws_categorie WHERE ncat_id = :id";
        $stmt = $this->db->prepare($query);            
        $stmt->bindParam(':id', $id);                                
        $stmt->execute();                
    }    
        
    protected function fetch(PDOStatement $stmt)
    {
        $data = $stmt->fetch(PDO::FETCH_ASSOC);
        
        return $data;
    }    
    
    protected function fetchAll(PDOStatement $stmt)
    {
        $data = $stmt->fetchAll(PDO::FETCH_ASSOC);
        
        return $data;
    }        
    
    protected function checkImage($array,$w,$h)
    {
        $allow = array("jpg","bmp","gif","png");
        $extensie = strtolower(substr($array["name"], -3));    
        if(!in_array($extensie,$allow))
            throw new InvalidImageException ('De extensie is niet toegestaan');        
        
        if(!empty($w) || !empty($h))
        {
            list($width, $height) = getimagesize($array['tmp_name']);    
            if(($w != $width) || ($h != $height) || (($w/$h) != ($width/$height)))
                throw new InvalidImageException ('De maten van de afbeelding zijn onjuist');    
        }
            
        return true;
    }    
    
    protected function storeImage(array $array)
    {
        $file = explode(".", $array["name"]);        
        $imgname = substr(md5($file[0].date("ymd")),0,12).'.'.$file[1];
        $uploadurl = "../images/news/".$imgname;
        if(is_uploaded_file($array['tmp_name'])) {
            move_uploaded_file($array['tmp_name'], $uploadurl);
        }
            
        return $imgname;        
    }        
}

/* General Functions */

function safeUrl($string, $koppelteken = '-') {
    $string = strtolower($string);
    $string = strip_tags($string);
    
    $string = preg_replace("#\&([a-z]{1})([a-z]{1,})\;#", "\\1", htmlentities($string)); // zet alle rare tekens om naar gewone tekens (het idee dankzij Webmakerij)
    $string = str_replace(" ", $koppelteken, $string); // zet spaties om in het koppelteken
    $string = preg_replace("#[^\w".preg_quote($koppelteken, "#")."]#", "", $string); // haal alle tekens zoals quotes, komma's en punten uit de string, behalve het koppelteken
    $string = trim($string, $koppelteken); // haal de koppeltekens aan de uiteinden van de string weg

    return $string; // geef m maar terug :)
}

function isUserAuthorized( $sectionlevel )
{
    // controleer of de sessie gegevens aanwezig zijn
    if( !isset( $_SESSION["USER"]["LEVEL"] ) )
    {
        // de sessie gegevens zijn niet aanwezig
        return false;
    }
 
    // haal het gebruikersniveau op (in integer formaat)
    $userlevel = intval( $_SESSION["USER"]["LEVEL"]  );
 
    // ok, hier gebeurd de werkelijke magic ;-) We gebruiken de bitwise AND operator!
    // controleer of de gebruiker toegang heeft tot de sectie
    return ( $userlevel & $sectionlevel ) > 0 ;
}
?>
 
PHP hulp

PHP hulp

22/12/2024 12:51:36
 
Jelmer -

Jelmer -

23/06/2010 23:56:55
Quote Anchor link
Paar kleine foutjes die ik tegenkom wanneer ik je code lees. Dit is zeg maar wat er in mij opkomt. Dat betekent niet dat het per definitie zo hoort hoor :)
- __construct mist nog public keyword.
- checkUsername & checkPassword zijn van die vraag-functies. Ik zou ze true/false laten teruggeven en public maken, misschien hernoemen naar isValidUsername, isValidPassword. Dan kan je ze later ook nog gebruiken bij bijvoorbeeld het valideren van een formuliertje.
- checkCombination zal een fout geven, omdat je PDOStatement::bindParam bij :pass niet een variabele geeft, maar de uitvoer van een functie. Dan kan bindParam niet gebruiken (zie references in PHP) en moet je bindValue pakken.
- NewsController::getNews() Hier kan je ook $this->db->query() in een keer aanroepen, prepare is een beetje overbodig. Maar er is wel wat voor te zeggen om prepare ivm consistentie te gebruiken :)
- NewsController::getCategories gooit een exception wanneer er geen categorieën zijn. Naar mijn idee is dat niet terecht, je vraagt gewoon "geef mij aaaaalle categorieën", nou, dat doet 'ie netjes. Dan is een lege array toch geen onverwacht gedrag waarvoor je een exception moet gooien?
- NewsController::getNewsCount geeft een array terug, terwijl je gewoon een nummer zou verwachten
- persoonlijk zou ik voor addNews, addCategory gaan in plaats van set*. Setten is ergens een nieuwe waarde aan toekennen, maar jij voegt iets toe, en overschrijft het oude niet. add is dan logischer niet?

Sowieso zou ik direct hierbij ook een klein stukje site maken dat deze classes gebruikt. Vaak is het uitdenken van een API veel gemakkelijker dan het lijkt. Dan heb je echt een hele mooie API, maar is hij gewoon totaal niet praktisch in gebruik. En dan schiet je je doel voorbij. Dat gaat heel gemakkelijk met OOP.

Maar volgens mij heb je het redelijk onder de knie zo. Dit is naar mijn idee een redelijk nette en praktische oplossing. Je maakt nog niet zoveel gebruik van objecten, dit is meer een verzameling functies, maar volgens mij is dat hier de beste oplossing. Verderop kom je nog wel wat meer in de knoei met objecten, wacht maar >:)
 
Roeltje M

Roeltje M

24/06/2010 00:21:23
Quote Anchor link
Bedankt voor je analyse Jelmer!
Ik zal alles veranderen in Add*. Moet ik dan update veranderen in Set? Of gewoon update laten?
Verder heb ik 'm al ergens online lopen (deze classes), maar stopte en liet het controleren voor ik verder ging.

Bij je punt "- checkCombination zal een fout geven" geef je aan bindValue te gebruiken, maar bindParam werkt ook gewoon.
En ik maak inderdaad weinig gebruik van objecten. Ik weet ook niet zo snel waar ik dat in praktijk toe kan passen.
Quote:
checkUsername & checkPassword zijn van die vraag-functies. Ik zou ze true/false
Is dat niet ik al doe? Als de if niet klopt, geeft ie true terug.
Gewijzigd op 24/06/2010 00:27:02 door Roeltje M
 
Jelmer -

Jelmer -

24/06/2010 00:30:20
Quote Anchor link
Roeltje M op 24/06/2010 00:21:23:
Bij je punt "- checkCombination zal een fout geven" geef je aan bindValue te gebruiken, maar bindParam werkt ook gewoon.

Hmm, dan is het zeker alleen een notice. Als je error_reporting(E_ALL) set, zou je het moeten zien.
Quote:
Quote:
checkUsername & checkPassword zijn van die vraag-functies. Ik zou ze true/false
Is dat niet ik al doe? Als de if niet klopt, geeft ie true terug.

Hij geeft altijd true terug. Als die if niet klopt, komt hij niet eens aan return toe. Stel dat ik een formuliertje zou willen schrijven dat die functies gebruikt om te checken, dan zou ik om ieder veld een try-catch blok moeten zetten om ervoor te zorgen dat 'ie twee fouten tegelijk kan vinden. Als je heel simpel zou doen:
Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
4
5
<?php
function checkUsername($username) {
    return strlen($username) > 3;
}
?>

dan zou je ook code kunnen schrijven als
Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
4
5
6
<?php
if(!$x->checkUsername($_POST...))
    echo 'dit veld is niet goed';
if(!$x->checkPassword($_POST...))
    echo 'en dit veld ook niet!';
?>

Nu moet je daar twee try-catch blokken omheen zetten, wat mij wat vreemd lijkt.
 
Roeltje M

Roeltje M

24/06/2010 00:42:50
Quote Anchor link
Ik nu (in een login class dus):
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
<?php
    public function login($username,$password)
    {
        $this->checkUsername($username);
        $this->checkPassword($password);
            
        if($user = $this->checkCombination($username,$password))
        {
            $_SESSION['USER']['ID'] = $user['user_id'];
            $_SESSION['USER']['NAME'] = $user['user_loginname'];    
            $_SESSION['USER']['LEVEL'] = $user['user_level'];                
            $_SESSION['USER']['LOGGEDIN'] = true;        
        }
        
        return true;
    }    
?>


De try+catch heb ik op de login.php pagina:

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
<?php
session_start();
include("../includes/config.inc.php");
$loggedin = false;    
if($_SERVER['REQUEST_METHOD'] == 'POST' && $_POST['form'] == 'login')
{
    try
    {
        $error = '';
        $session = new UserSessionController($db);
        $session->login($_POST['user'],$_POST['pass']);
        if(isUserAuthorized(ADMIN_PANEL))
        {
            $loggedin = true;        
            header("Refresh:2 ; URL=admin.php");
        }
        else
        {
            $error = "U heeft geen rechten om dit te bezoeken";
            session_destroy();
        }
    }
    catch(Exception $e)
    {
        $error = $e->getMessage();            
    }        
    
}
?>


De functie hoeft alleen veranderd te worden? Naar

Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
4
5
<?php
function checkUsername($username) {
    return strlen($username) > 3;
}
?>


of iets met if else erin ? (voor duidelijkheid)
Gewijzigd op 24/06/2010 00:43:46 door Roeltje M
 
Jelmer -

Jelmer -

24/06/2010 08:25:55
Quote Anchor link
De if-else komt dan in de login functie. Vanuit daar mag je op zich wel een exception gooien lijkt mij.

Al die check functies daar ben je al heel voorzichtig. Zoals de naam zegt verwacht je dat het goed of fout is wat je erin stopt. Bij login verwacht je gewoon dat je wordt ingelogd. Anders had je het wel checkLogin genoemd. Een exception gooi je wanneer er iets onverwachts optreedt.
 
Moe BE

Moe BE

24/06/2010 11:04:32
Quote Anchor link
Even ter zijde: ik zou afbeeldingen die geupload worden controleren op MIME-type.
 
Roeltje M

Roeltje M

24/06/2010 13:34:28
Quote Anchor link
Hoe moet ik de login functie dan maken? Zo?

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
<?php
    public function login($username,$password)
    {
        if($this->checkUsername($username) && $this->checkPassword($password))
        {          
            if($user = $this->checkCombination($username,$password))
            {
                $_SESSION['USER']['ID'] = $user['user_id'];
                $_SESSION['USER']['NAME'] = $user['user_loginname'];    
                $_SESSION['USER']['LEVEL'] = $user['user_level'];                
                $_SESSION['USER']['LOGGEDIN'] = true;        
                return true;                
            }
        }
        else
        {
            // exception hier
            return false;
        }
      
    }    
?>
 
Jelmer -

Jelmer -

25/06/2010 08:19:44
Quote Anchor link
Als je specifiek checkt of ze niet correct zijn, dus op !checkUsername OR !checkPassword, hoef je niet allemaal if-lussen in elkaar te schrijven. Misschien is dit wat overzichtelijker:
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
<?php
    public function login($username,$password)
    {
        if(!$this->checkUsername($username))
             throw new InvalidUsernameException();
        
        if(!$this->checkPassword($password))
            throw new InvalidPasswordException();
        
        $user = $this->checkCombination($username,$password);
        
        if(!$user)
            return false;
        
        $_SESSION['USER']['ID'] = $user['user_id'];
        $_SESSION['USER']['NAME'] = $user['user_loginname'];
        $_SESSION['USER']['LEVEL'] = $user['user_level'];
        $_SESSION['USER']['LOGGEDIN'] = true;
        
        return true;
    }
?>
 



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.