Feedback op mijn OOP script
Ik ben nieuw in de OOP hoek van PHP en heb iets geschreven wat voor mij wel werkt, maar weet niet of dit goed en op de juiste manier is.
Mijn school heeft mij wel wat over OOP verteld maar was allemaal erg onduidelijk.
Ik heb dit geschreven samen met behulp van een tutorial "http://net.tutsplus.com/tutorials/php/object-oriented-php-for-beginners/"
Zouden jullie mij tips e.d kunnen geven op dit script en ook zeggen wat ik "fout" doe of misschien onlogisch.
Geen afkraak dingen of modder gooien aub, een kind die net gaat schaatsen sta je ook niet ui te lachen op het ijs.
Alvast bedankt!
PHP CLASS:
Code (php)
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
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
<?php
/**
* Menu functies en classes
*
* Voor het installeren van het menu zijn een aantal
* checks en configuraties nodig. Deze checks worden
* op een "INSTALL" pagina gedaan en getoont. De daad-
* werkelijke checks worden dmv een class of functie
* uitgevoerd
*
* @author Rick de Graaff
* @date 18-01-2012
*
*/
class checkModule {
/**
* Vars voor de class
*
* @param s_Module = modulenaam
* @param a_Tables = array met tabelnamen
* @param a_Files = array met bestandsnamen
* @param a_Folders = array met mapnamen
* @param i_Integer = teller zodat elke tabel een eigen nummer krijgt (tijdelijk nummer)
* @param a_Errors = array waar errors in kunnen staan (deze gebruik ik nog niet)
* @param s_Debug = Als de debug aanstaat word er bij de __destruct een printr getoont met de errors
*
*/
public $s_Module;
public $a_Tables;
public $a_Files;
public $a_Folders;
public $a_Errors;
public $s_Debug;
/* Constructor */
public function __construct($sModule, $aTables, $aFiles, $aFolders, $sDebug){
$this->s_Module = $sModule;
$this->a_Tables = $aTables;
$this->a_Files = $aFiles;
$this->a_Folders = $aFolders;
$this->a_Errors = array();
$this->s_Debug = $sDebug;
// Laat zien om wat voor module het gaat
echo '<h2>MODULE '.strtoupper($this->s_Module).'</h2>'."\n";
echo '<p>Kijk welke tabellen en/of de module al geïnstalleerd staat. Alle tabellen zijn nodig in een module.<br />Vink anders aan wat je wilt installeren.</p>'."\n";
echo '<div class="background">'."\n\n";
echo '<h3>Module actief</h3>'."\n";
}
/* Destructor */
public function __destruct(){
// Sluit de background div
echo '</div>'."\n\n";
if($this->s_Debug == true){
printr($this->a_Errors);
}
}
/**
* Controleren of module bestaat
*
*/
public function moduleCheck(){
// Start van de UL LIST
echo '<div class="formlist-nodrag">'."\n";
echo '<ul>'."\n";
$qCheckInstalled = 'SELECT * FROM modules WHERE moduleNaam = "'.$this->s_Module.'" LIMIT 1';
$mCheckInstalled = mysql_query($qCheckInstalled) or die($sError = '<div><strong>qCheckInstalled error: </strong><br />'.$qCheckInstalled.'<br /><pre></div>'.mysql_error().'</pre>');
$fCheckInstalled = mysql_fetch_array($mCheckInstalled);
$cCheckInstalled = mysql_num_rows($mCheckInstalled);
// Parse voor de mysql error
if(!$mCheckInstalled){ echo $sError;}
// Melding of de module al geïnstalleerd is
if($cCheckInstalled >= 1){
echo '<li><img src="images/icons/silk/accept.png" alt="Wel geïnstalleerd" title="Wel geïnstalleerd" /> Module '.$this->s_Module.' is actief</li>';
} elseif($cCheckInstalled == 0){
echo '<li><img src="images/icons/silk/exclamation.png" alt="Nog niet geïnstalleerd" title="Nog niet geïnstalleerd" /> Module '.$this->s_Module.' is nog <strong>niet</strong> geïnstalleerd <span><input type="checkbox" name="'.stripStr(strtolower($this->s_Module)).'" /></span></li>';
// Error aanmaken in de "error array"
}
// Einde van de UL LIST
echo '</ul>'."\n";
echo '</div>'."\n\n";
}
/**
* Functies en uitvoeringen voor tabels checks
*
*/
public function tablesCheck(){
// Start van de UL LIST
echo '<br /><h3>Tabellen</h3>'."\n";
echo '<div class="formlist-nodrag">'."\n";
echo '<ul>'."\n";
/* Doorloop alle tables die in de array staan */
foreach($this->a_Tables as $sTable){
$qCheckTables = 'SHOW TABLES LIKE "'.$sTable.'"';
$mCheckTables = mysql_query($qCheckTables) or die($sError = '<div><strong>qCheckTables error: </strong><br />'.$qCheckTables.'<br /><pre></div>'.mysql_error().'</pre>');
$fCheckTables = mysql_fetch_array($mCheckTables);
if($fCheckTables[0] == $sTable){
echo '<li><img src="images/icons/silk/database.png" alt="Wel geïnstalleerd" title="Wel geïnstalleerd" /> Tabel '.$sTable.' bestaat al</li>';
} else {
echo '<li><img src="images/icons/silk/database_error.png" alt="Nog niet geïnstalleerd" title="Nog niet geïnstalleerd" /> Tabel '.$sTable.' is niet aangemaakt <span><input type="checkbox" name="'.stripStr(strtolower($sTable)).'" /></span></li>';
}
// Parse voor de mysql error
if(!$mCheckInstalled){ echo $sError;}
}
// Einde van de UL LIST
echo '</ul>'."\n";
echo '</div>'."\n\n";
}
/**
* Mappen check
*
* Check of de mappen die nodig zijn voor deze module aanwezig zijn
*/
public function foldersCheck(){
// Start van de UL LIST
echo '<br /><h3>Mappen</h3>'."\n";
echo '<div class="formlist-nodrag">'."\n";
echo '<ul>'."\n";
/* Waardes 1 voor 1 uit de array halen en controleren */
foreach($this->a_Folders as $sIndexKey => $sFolder){
if(is_dir($sFolder)){
echo '<li><img src="images/icons/silk/folder.png" alt="Folder bestaat" title="Folder bestaat" /> Map "'.$sIndexKey.'" bestaat al</li>';
} elseif(!is_dir($sFolder)){
echo '<li><img src="images/icons/silk/folder_error.png" alt="Folder niet aanwezig" title="Folder niet aanwezig" /> Map "'.$sIndexKey.'" is niet aangemaakt <span><input type="checkbox" name="'.stripStr(strtolower($sIndexKey)).'" /></span></li>';
}
}
// Einde van de UL LIST
echo '</ul>'."\n";
echo '</div>'."\n\n";
}
/**
* Bestands check
*
* Een array die word uigelezen en checkt of de bestanden wel aanwezig zijn
*/
public function filesCheck(){
// Start van de UL LIST
echo '<br /><h3>Bestanden</h3>'."\n";
echo '<div class="formlist-nodrag">'."\n";
echo '<ul>'."\n";
/* Waardes 1 voor 1 uit de array halen en controleren */
foreach($this->a_Files as $sIndexKey => $sFile){
if(file_exists($sFile)){
echo '<li><img src="images/icons/silk/script.png" alt="Bestand bestaat" title="Bestand bestaat" /> Bestand "'.$sIndexKey.'" bestaat al</li>';
} elseif(!file_exists($sFile)){
echo '<li><img src="images/icons/silk/script_error.png" alt="Bestand niet aanwezig" title="Bestand niet aanwezig" /> Bestand "'.$sIndexKey.'" is niet aangemaakt <span><input type="checkbox" name="'.stripStr($sIndexKey).'" /></span></li>';
}
}
// Einde van de UL LIST
echo '</ul>'."\n";
echo '</div>'."\n\n";
}
}
?>
/**
* Menu functies en classes
*
* Voor het installeren van het menu zijn een aantal
* checks en configuraties nodig. Deze checks worden
* op een "INSTALL" pagina gedaan en getoont. De daad-
* werkelijke checks worden dmv een class of functie
* uitgevoerd
*
* @author Rick de Graaff
* @date 18-01-2012
*
*/
class checkModule {
/**
* Vars voor de class
*
* @param s_Module = modulenaam
* @param a_Tables = array met tabelnamen
* @param a_Files = array met bestandsnamen
* @param a_Folders = array met mapnamen
* @param i_Integer = teller zodat elke tabel een eigen nummer krijgt (tijdelijk nummer)
* @param a_Errors = array waar errors in kunnen staan (deze gebruik ik nog niet)
* @param s_Debug = Als de debug aanstaat word er bij de __destruct een printr getoont met de errors
*
*/
public $s_Module;
public $a_Tables;
public $a_Files;
public $a_Folders;
public $a_Errors;
public $s_Debug;
/* Constructor */
public function __construct($sModule, $aTables, $aFiles, $aFolders, $sDebug){
$this->s_Module = $sModule;
$this->a_Tables = $aTables;
$this->a_Files = $aFiles;
$this->a_Folders = $aFolders;
$this->a_Errors = array();
$this->s_Debug = $sDebug;
// Laat zien om wat voor module het gaat
echo '<h2>MODULE '.strtoupper($this->s_Module).'</h2>'."\n";
echo '<p>Kijk welke tabellen en/of de module al geïnstalleerd staat. Alle tabellen zijn nodig in een module.<br />Vink anders aan wat je wilt installeren.</p>'."\n";
echo '<div class="background">'."\n\n";
echo '<h3>Module actief</h3>'."\n";
}
/* Destructor */
public function __destruct(){
// Sluit de background div
echo '</div>'."\n\n";
if($this->s_Debug == true){
printr($this->a_Errors);
}
}
/**
* Controleren of module bestaat
*
*/
public function moduleCheck(){
// Start van de UL LIST
echo '<div class="formlist-nodrag">'."\n";
echo '<ul>'."\n";
$qCheckInstalled = 'SELECT * FROM modules WHERE moduleNaam = "'.$this->s_Module.'" LIMIT 1';
$mCheckInstalled = mysql_query($qCheckInstalled) or die($sError = '<div><strong>qCheckInstalled error: </strong><br />'.$qCheckInstalled.'<br /><pre></div>'.mysql_error().'</pre>');
$fCheckInstalled = mysql_fetch_array($mCheckInstalled);
$cCheckInstalled = mysql_num_rows($mCheckInstalled);
// Parse voor de mysql error
if(!$mCheckInstalled){ echo $sError;}
// Melding of de module al geïnstalleerd is
if($cCheckInstalled >= 1){
echo '<li><img src="images/icons/silk/accept.png" alt="Wel geïnstalleerd" title="Wel geïnstalleerd" /> Module '.$this->s_Module.' is actief</li>';
} elseif($cCheckInstalled == 0){
echo '<li><img src="images/icons/silk/exclamation.png" alt="Nog niet geïnstalleerd" title="Nog niet geïnstalleerd" /> Module '.$this->s_Module.' is nog <strong>niet</strong> geïnstalleerd <span><input type="checkbox" name="'.stripStr(strtolower($this->s_Module)).'" /></span></li>';
// Error aanmaken in de "error array"
}
// Einde van de UL LIST
echo '</ul>'."\n";
echo '</div>'."\n\n";
}
/**
* Functies en uitvoeringen voor tabels checks
*
*/
public function tablesCheck(){
// Start van de UL LIST
echo '<br /><h3>Tabellen</h3>'."\n";
echo '<div class="formlist-nodrag">'."\n";
echo '<ul>'."\n";
/* Doorloop alle tables die in de array staan */
foreach($this->a_Tables as $sTable){
$qCheckTables = 'SHOW TABLES LIKE "'.$sTable.'"';
$mCheckTables = mysql_query($qCheckTables) or die($sError = '<div><strong>qCheckTables error: </strong><br />'.$qCheckTables.'<br /><pre></div>'.mysql_error().'</pre>');
$fCheckTables = mysql_fetch_array($mCheckTables);
if($fCheckTables[0] == $sTable){
echo '<li><img src="images/icons/silk/database.png" alt="Wel geïnstalleerd" title="Wel geïnstalleerd" /> Tabel '.$sTable.' bestaat al</li>';
} else {
echo '<li><img src="images/icons/silk/database_error.png" alt="Nog niet geïnstalleerd" title="Nog niet geïnstalleerd" /> Tabel '.$sTable.' is niet aangemaakt <span><input type="checkbox" name="'.stripStr(strtolower($sTable)).'" /></span></li>';
}
// Parse voor de mysql error
if(!$mCheckInstalled){ echo $sError;}
}
// Einde van de UL LIST
echo '</ul>'."\n";
echo '</div>'."\n\n";
}
/**
* Mappen check
*
* Check of de mappen die nodig zijn voor deze module aanwezig zijn
*/
public function foldersCheck(){
// Start van de UL LIST
echo '<br /><h3>Mappen</h3>'."\n";
echo '<div class="formlist-nodrag">'."\n";
echo '<ul>'."\n";
/* Waardes 1 voor 1 uit de array halen en controleren */
foreach($this->a_Folders as $sIndexKey => $sFolder){
if(is_dir($sFolder)){
echo '<li><img src="images/icons/silk/folder.png" alt="Folder bestaat" title="Folder bestaat" /> Map "'.$sIndexKey.'" bestaat al</li>';
} elseif(!is_dir($sFolder)){
echo '<li><img src="images/icons/silk/folder_error.png" alt="Folder niet aanwezig" title="Folder niet aanwezig" /> Map "'.$sIndexKey.'" is niet aangemaakt <span><input type="checkbox" name="'.stripStr(strtolower($sIndexKey)).'" /></span></li>';
}
}
// Einde van de UL LIST
echo '</ul>'."\n";
echo '</div>'."\n\n";
}
/**
* Bestands check
*
* Een array die word uigelezen en checkt of de bestanden wel aanwezig zijn
*/
public function filesCheck(){
// Start van de UL LIST
echo '<br /><h3>Bestanden</h3>'."\n";
echo '<div class="formlist-nodrag">'."\n";
echo '<ul>'."\n";
/* Waardes 1 voor 1 uit de array halen en controleren */
foreach($this->a_Files as $sIndexKey => $sFile){
if(file_exists($sFile)){
echo '<li><img src="images/icons/silk/script.png" alt="Bestand bestaat" title="Bestand bestaat" /> Bestand "'.$sIndexKey.'" bestaat al</li>';
} elseif(!file_exists($sFile)){
echo '<li><img src="images/icons/silk/script_error.png" alt="Bestand niet aanwezig" title="Bestand niet aanwezig" /> Bestand "'.$sIndexKey.'" is niet aangemaakt <span><input type="checkbox" name="'.stripStr($sIndexKey).'" /></span></li>';
}
}
// Einde van de UL LIST
echo '</ul>'."\n";
echo '</div>'."\n\n";
}
}
?>
PHP CODE OM CLASS AAN TE ROEPEN:
Code (php)
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
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
<?php
/*
* MODULE CHECKEN
*
* Bekijk of alle bestanden en tabellen juist zijn en ook bestaan.
* Als deze niet bestaan krijg je een nette melding en kan je ze
* alsnog laten maken door het CMS
*
* @param, alle variabelen staan hieronder die nodig zijn om alles
* te controleren voor de module (de inhoud van de variabelen verschilt per module).
*
* Opbouw module check:
*
* - Array met Tabellen
* - Array met Bestanden
* - Array met Mappen
* - Variabele met "module naam"
*
*
*
*/
$aTables = array('menu', 'menu_talen', 'jaja');
$aFiles = array(".htaccess" => ''.$sExtSub.'.htaccess', ".index.asp" => ''.$sExtSub.'.index.asp');
$aFolders = array("pages" => ''.$sExtSub.'pages/', "bla" => ''.$sExtSub.'bla/',);
$sModule = 'menu';
// Nieuwe module check aanmaken
$sMenu = new checkModule($sModule, $aTables, $aFiles, $aFolders, false);
// Controleer of module actief staat in de DB
echo $sMenu->moduleCheck();
// Controle of de tabellen wel aanwezig zijn die nodig zijn
echo $sMenu->tablesCheck();
// Controle of de tabellen wel aanwezig zijn die nodig zijn
echo $sMenu->foldersCheck();
// Controle of de tabellen wel aanwezig zijn die nodig zijn
echo $sMenu->filesCheck();
// Sluit de class af en check dmv de destructor voor errors
unset($sMenu);
?>
/*
* MODULE CHECKEN
*
* Bekijk of alle bestanden en tabellen juist zijn en ook bestaan.
* Als deze niet bestaan krijg je een nette melding en kan je ze
* alsnog laten maken door het CMS
*
* @param, alle variabelen staan hieronder die nodig zijn om alles
* te controleren voor de module (de inhoud van de variabelen verschilt per module).
*
* Opbouw module check:
*
* - Array met Tabellen
* - Array met Bestanden
* - Array met Mappen
* - Variabele met "module naam"
*
*
*
*/
$aTables = array('menu', 'menu_talen', 'jaja');
$aFiles = array(".htaccess" => ''.$sExtSub.'.htaccess', ".index.asp" => ''.$sExtSub.'.index.asp');
$aFolders = array("pages" => ''.$sExtSub.'pages/', "bla" => ''.$sExtSub.'bla/',);
$sModule = 'menu';
// Nieuwe module check aanmaken
$sMenu = new checkModule($sModule, $aTables, $aFiles, $aFolders, false);
// Controleer of module actief staat in de DB
echo $sMenu->moduleCheck();
// Controle of de tabellen wel aanwezig zijn die nodig zijn
echo $sMenu->tablesCheck();
// Controle of de tabellen wel aanwezig zijn die nodig zijn
echo $sMenu->foldersCheck();
// Controle of de tabellen wel aanwezig zijn die nodig zijn
echo $sMenu->filesCheck();
// Sluit de class af en check dmv de destructor voor errors
unset($sMenu);
?>
Edit:
ter aanvulling, objecten gebruik je om de logica te onderscheiden van je visuele representatie. Er zijn uitzonderingen hier op, maar eigenlijk hoor je die te negeren. Je moet er altijd voor zorgen dat objecten logic doen, en view files ( templates etc ) de representatie.
Ik zie ook dat je hele menu structuren hanteert in je object, dat zou ik niet doen. Dat zou ik in een template bestand gooien en je object dus gewoon een array laten terug geven. Vervolgens (recursief) door je array lopen en dat als output genereren.
ter aanvulling, objecten gebruik je om de logica te onderscheiden van je visuele representatie. Er zijn uitzonderingen hier op, maar eigenlijk hoor je die te negeren. Je moet er altijd voor zorgen dat objecten logic doen, en view files ( templates etc ) de representatie.
Ik zie ook dat je hele menu structuren hanteert in je object, dat zou ik niet doen. Dat zou ik in een template bestand gooien en je object dus gewoon een array laten terug geven. Vervolgens (recursief) door je array lopen en dat als output genereren.
Gewijzigd op 02/02/2012 12:20:33 door Pieter Jansen
Er zitten while loops in de functies, ik denk dat dit daardoor komt.
Oplossing? Alles parsen naar een variabele (binnen de functie) en dan parsen?
Toevoeging op 02/02/2012 12:23:35:
Das wel een hele goeie oplossing ja. Een template laden en daar de waardes in laten plaatsen.
(y) Hier heb ik wat aan! :D
Kijk het werkt wel, maar wist niet of ik het goed deed..
Hartstikke bedankt!
Meerdere feedbacks zijn welkom hoor! Lees ze allemaal :)
Code (php)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
<?php
public function filesCheck(){
$a = array();
/* Waardes 1 voor 1 uit de array halen en controleren */
foreach($this->a_Files as $sIndexKey => $sFile){
if(file_exists($sFile)){
$a[$sKindexKey] = array($sFile, true);
} elseif(!file_exists($sFile)){
$a[$sKindexKey] = array($sFile, false);
}
}
return $a;
}
?>
public function filesCheck(){
$a = array();
/* Waardes 1 voor 1 uit de array halen en controleren */
foreach($this->a_Files as $sIndexKey => $sFile){
if(file_exists($sFile)){
$a[$sKindexKey] = array($sFile, true);
} elseif(!file_exists($sFile)){
$a[$sKindexKey] = array($sFile, false);
}
}
return $a;
}
?>
Gewijzigd op 02/02/2012 12:30:19 door Pieter Jansen
Maar wat jij zegt met een template laden, is 100x makkelijker en fijner..
Gaat helemaal goed komen!
En bedankt voor je voorbeeld scriptje
Een constructor heeft geen return (je krijgt het object al terug) maar daarin hoor je verder ook geen echo te plaatsen. Bij de andere methode moet als je een tekst terug geeft die met echo wel te laten zien. Anders gaat er iets anders fout.
Dan verder over je code, als je het commentaar direct boven je functie zet (i.p.v.) een lege regel er tussen kan het opgepakt worden door tools als doxygen om je documentatie te genereren.
Kijk ook even naar je inspringen, je spring wel heel snel in. (Naar mijn mening)
Leuk dat je tijd wilt steken in OOP en dat hier duidelijk toont. Daar kunnen anderen veel van leren. Over een jaar wil je niet anders ;)