OOP Beginner, ben ik goed op weg?
Ik ben nog maar eens herbegonnen met het leren van OOP. Omdat OOP zeer moeilijk is in het begin omdat het toch een soort andere werkwijze is wil ik hier even mijn eerste OOP script plaatsen. Tip, commentaar of verbeteringen hoor ik graag! Het script is eigenlijk zeer simpel. Je kan in de URL je voor- en achternaam + je land typen (d.m.v. ?firstname=&lastname=&country=). Het script zorgt er dan voor dat ik alles zeer gemakkelijk kan printen door de get_all() functie uit te voeren.
Heel nuttig is dit script niet, maar het gaat hem hier om de training die ik er van krijg. Je zal zien dat de functies get_name en get_country niet private zijn. Het zou inderdaad wel beter/veiliger zijn dit te doen. Maar de reden waarom dit niet zo is, is omdat ik die functies dan apart kan oproepen. Als ik bijvoorbeeld alleen de naam wil verkrijgen.
index.php
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
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
<!DOCTYPE HTML>
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<title></title>
<?php
require_once('lib/class_lib.php');
?>
</head>
<body>
<?php
$person = new person();
if(isset($_GET['firstname']) && isset($_GET['lastname'])) $person->set_name($_GET['firstname'], $_GET['lastname']);
else if(isset($_GET['firstname'])) $person->set_name($_GET['firstname'], 'None');
else if(isset($_GET['lastname'])) $person->set_name('None', $_GET['lastname']);
else $person->set_name('None', 'None');
if(isset($_GET['country'])) $person->set_country($_GET['country']);
else $person->set_country('None');
echo $person->get_all();
?>
</body>
</html>
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<title></title>
<?php
require_once('lib/class_lib.php');
?>
</head>
<body>
<?php
$person = new person();
if(isset($_GET['firstname']) && isset($_GET['lastname'])) $person->set_name($_GET['firstname'], $_GET['lastname']);
else if(isset($_GET['firstname'])) $person->set_name($_GET['firstname'], 'None');
else if(isset($_GET['lastname'])) $person->set_name('None', $_GET['lastname']);
else $person->set_name('None', 'None');
if(isset($_GET['country'])) $person->set_country($_GET['country']);
else $person->set_country('None');
echo $person->get_all();
?>
</body>
</html>
lib/class_lib.php
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
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
<?php
// CLASS: person
class person
{
private $first_name;
private $last_name;
private $country;
function set_name($new_first_name, $new_last_name)
{
$this->first_name = $new_first_name;
$this->last_name = $new_last_name;
}
function set_country($new_country)
{
$this->country = $new_country;
}
function get_all()
{
echo 'Your name is '.$this->get_name(0);
echo '<br />Your first name is '.$this->get_name(1);
echo '<br />Your last name is '.$this->get_name(2);
echo '<br />Your country is '.$this->get_country();
}
function get_name($option1)
{
switch($option1)
{
case 0:
return $this->first_name.' '.$this->last_name;
break;
case 1:
return $this->first_name;
break;
case 2:
return $this->last_name;
break;
default:
return $this->first_name.' '.$this->last_name;
break;
}
}
function get_country()
{
return $this->country;
}
}
?>
// CLASS: person
class person
{
private $first_name;
private $last_name;
private $country;
function set_name($new_first_name, $new_last_name)
{
$this->first_name = $new_first_name;
$this->last_name = $new_last_name;
}
function set_country($new_country)
{
$this->country = $new_country;
}
function get_all()
{
echo 'Your name is '.$this->get_name(0);
echo '<br />Your first name is '.$this->get_name(1);
echo '<br />Your last name is '.$this->get_name(2);
echo '<br />Your country is '.$this->get_country();
}
function get_name($option1)
{
switch($option1)
{
case 0:
return $this->first_name.' '.$this->last_name;
break;
case 1:
return $this->first_name;
break;
case 2:
return $this->last_name;
break;
default:
return $this->first_name.' '.$this->last_name;
break;
}
}
function get_country()
{
return $this->country;
}
}
?>
Gewijzigd op 11/06/2011 11:47:08 door Ruben Portier
Ook de functie get_all is overbodig. Op zich zou het wel kunnen. maar het is niet de bedoeling dat je via methode in een object html code gaat terug geven.
Gewijzigd op 11/06/2011 12:30:05 door Moe BE
Ok bedankt! Dus ik zal first en lastname koppelen tot name eigenlijk. En bij get_all zou ik alle gegevens in een array kunne plaatsen?
Ook is get_name() een beetje apart.
Een functie moet 1 ding uitvoeren. Dan is deze beter te herbruiken (het DRY principe, Don't Repeat Yourself).
Maak er dan 3 methoden van:
Code (php)
Leer jezelf aan om methoden een scope te geven. D.w.z. private, public of protected. Ik zie namelijk dat je de velden wel netjes op private zet.
Verder is een klasse aanmaken en er een instantie van maken niet bepaald OOP, maar gewoon werken met een klasse ;)
Ik zie dat je nu eerst een object aanmaakt en vervolgens met de public set-methoden de velden instelt. Al eens gekeken naar constructors? Dat is een methode die aangeroepen wordt bij het instantieren. Daar kun je ook direct parameters aan geven.
Code (php)
Gewijzigd op 11/06/2011 13:18:43 door Erik van de Locht
Moe BE op 11/06/2011 12:19:37:
Ook de functie get_all is overbodig. Op zich zou het wel kunnen. maar het is niet de bedoeling dat je via methode in een object html code gaat terug geven.
Op zich is daar niets mis mee, zo lang dat maar de bedoeling is van het object. Hier is het natuurlijk niet (echt) goed. Toch is het niet echt probleem wanneer je alle domain objects (personen, huizen e.d.) ook de verantwoordelijkheid geeft zichzelf in HTML weer te geven. De naam get_all() is dan wel verkeerd gekozen.
Pim - op 11/06/2011 15:38:10:
Op zich is daar niets mis mee, zo lang dat maar de bedoeling is van het object. Hier is het natuurlijk niet (echt) goed. Toch is het niet echt probleem wanneer je alle domain objects (personen, huizen e.d.) ook de verantwoordelijkheid geeft zichzelf in HTML weer te geven. De naam get_all() is dan wel verkeerd gekozen.
Moe BE op 11/06/2011 12:19:37:
Ook de functie get_all is overbodig. Op zich zou het wel kunnen. maar het is niet de bedoeling dat je via methode in een object html code gaat terug geven.
Op zich is daar niets mis mee, zo lang dat maar de bedoeling is van het object. Hier is het natuurlijk niet (echt) goed. Toch is het niet echt probleem wanneer je alle domain objects (personen, huizen e.d.) ook de verantwoordelijkheid geeft zichzelf in HTML weer te geven. De naam get_all() is dan wel verkeerd gekozen.
Getall kan mooi vervangen worden door tostring: language.oop5.magic#language.oop5.magic.tostring
Wijzigingen:
Functies zijn nu public.
De voor en achternaam zijn samengevoegt tot naam.
En wat kleine wijzigingen.
Ik zal zo nog even de constructor aanmaken, dat is inderdaad een goed idee! Ik begin het nut van OOP al wat door te krijgen. Bedankt voor alle hulp alvast!
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
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
// CLASS: person
class person
{
private $name;
private $country;
public function set_name($new_name)
{
$this->name = $new_name;
}
public function set_country($new_country)
{
$this->country = $new_country;
}
public function get_name()
{
return $this->name;
}
public function get_country()
{
return $this->country;
}
}
?>
// CLASS: person
class person
{
private $name;
private $country;
public function set_name($new_name)
{
$this->name = $new_name;
}
public function set_country($new_country)
{
$this->country = $new_country;
}
public function get_name()
{
return $this->name;
}
public function get_country()
{
return $this->country;
}
}
?>
EDIT: Ik heb het script nu nog wat bijgewerkt. Ik denk dat het nu wel goed is zoals OOP hoort te zijn. Maar toch post ik nog even de code. Graag hoor ik reacties! Bedankt alvast!
index.php
Code (php)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
<!DOCTYPE HTML>
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<title></title>
<?php
require_once('lib/class_lib.php');
?>
</head>
<body>
<?php
$person = new person('Ruben Portier','Belgium');
if(isset($_GET['name'])) $person->set_name($_GET['name']);
if(isset($_GET['country'])) $person->set_country($_GET['country']);
echo $person->get_name();
echo '<br />';
echo $person->get_country();
?>
</body>
</html>
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<title></title>
<?php
require_once('lib/class_lib.php');
?>
</head>
<body>
<?php
$person = new person('Ruben Portier','Belgium');
if(isset($_GET['name'])) $person->set_name($_GET['name']);
if(isset($_GET['country'])) $person->set_country($_GET['country']);
echo $person->get_name();
echo '<br />';
echo $person->get_country();
?>
</body>
</html>
lib/class_lib.php
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
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: person
class person
{
private $name;
private $country;
public function __construct($new_name, $new_country)
{
$this->name = $new_name;
$this->country = $new_country;
}
public function set_name($new_name)
{
$this->name = $new_name;
}
public function set_country($new_country)
{
$this->country = $new_country;
}
public function get_name()
{
return $this->name;
}
public function get_country()
{
return $this->country;
}
}
?>
// CLASS: person
class person
{
private $name;
private $country;
public function __construct($new_name, $new_country)
{
$this->name = $new_name;
$this->country = $new_country;
}
public function set_name($new_name)
{
$this->name = $new_name;
}
public function set_country($new_country)
{
$this->country = $new_country;
}
public function get_name()
{
return $this->name;
}
public function get_country()
{
return $this->country;
}
}
?>
Gewijzigd op 11/06/2011 18:39:08 door Ruben Portier
Sorry, voor en achternaam zou ik gescheiden houden, ik had niet gezien dat je daar 4 apparte parameters voor had. Ik zou hier dus ook 2 aparte methodes voor aanmaken. getFirstName, setFirstName, getLastName, setLastName. Een overkoepelende methode is dan nog mogelijk, maar ik zou daar niet voor kiezen.
@Moe BE: Bedankt! Ik vond het trouwens ook al raar waarom ik de voor- en achternaam niet gescheiden zou nemen. Ik zal dit even aanpassen. Zit de code voor de rest goed in elkaar? Ook qua beveiliging enzo? Want ik ben nog maar OOP beginner.
Ik vind het zelf niet zo handig om functionaliteit alleen met een magic methode aan te bieden, omdat deze vaak niet in een API doc staan of gevonden kunnen worden. Zie bijv SimpleXML, die veel functies alleen magisch aanbiedt, zodat de API documentatie heel onduidelijk is.
Zelf map ik vaak magische methoden naar gewone methoden.
@Ruben
Cruciaal, en ook heel lastig, is hoe je nu je persoon in een DB wilt gaan opslaan. Heb je al een idee? Zolang je niets opslaat, is je code (in het algemeen) veilig.
Toevoeging op 13/06/2011 21:43:55:
Kijk ook eens naar camelCase, vind ik er fijn werken.
Pim - op 13/06/2011 21:42:33:
@Karl
Ik vind het zelf niet zo handig om functionaliteit alleen met een magic methode aan te bieden, omdat deze vaak niet in een API doc staan of gevonden kunnen worden. Zie bijv SimpleXML, die veel functies alleen magisch aanbiedt, zodat de API documentatie heel onduidelijk is.
Zelf map ik vaak magische methoden naar gewone methoden.
(...)
Ik vind het zelf niet zo handig om functionaliteit alleen met een magic methode aan te bieden, omdat deze vaak niet in een API doc staan of gevonden kunnen worden. Zie bijv SimpleXML, die veel functies alleen magisch aanbiedt, zodat de API documentatie heel onduidelijk is.
Zelf map ik vaak magische methoden naar gewone methoden.
(...)
De toString zou gewoon de een getter voor de voornaam, achternaam en weetikveelwat aanroepen. Vind ik persoonlijk heel mooi als je gewoon een persoon kan echo'en.
Dat het niet goed in de docs staat ligt niet aan mij. Ik documenteer ze altijd wel. Ze staan altijd in mijn klassediagram.
Code (php)
1
2
3
4
5
6
7
8
9
10
11
12
13
2
3
4
5
6
7
8
9
10
11
12
13
<?php
public function setVal($sType,$sValue) {
$this->$sType = $sValue;
}
// Dan kun je het volgende doen:
$person->setVal('name','Bas Matthee');
$person->setVal('country','Nederland');
?>
public function setVal($sType,$sValue) {
$this->$sType = $sValue;
}
// Dan kun je het volgende doen:
$person->setVal('name','Bas Matthee');
$person->setVal('country','Nederland');
?>
Wel controleren of het type bestaat ..
Scherp! Maar was even als voorbeeld bedoeld.
Sterker nog, zelfs gewone getters en setters worden als een afbreuk aan de OOP-principes gezien. Deze (vrij complexe) post beschrijft hoe je er van af kan komen.
Daar heb je helemaal gelijk in, maar het was gewoon een toevoeging op Bas.
Mooi artikel!
@Bas.
;-)
Gewijzigd op 20/06/2011 18:51:41 door Niels K
Ik vraag me trouwens af: van de professionals, bij wie is het op werk 'te slordig' om getters en setters te gebruiken? Dat aantal is vast niet groot.
Wij gebruiken altijd getters en setters eigenlijk.
Inderdaad, en ik vind het nogal een voordeel dat wanneer je een setter gebruikt, je op een later tijdstip (verder in het project) makkelijk alsnog je data kan manipuleren zonder daarvoor ook alle scripts waarin je de waarde "set" nog eens aan moet gaan passen.
Kan je hiervan een voorbeeld geven?
Nou, stel ik heb een class waarin ik variabelen set via methods en ik heb een aantal scripts dat middels user-input (oid.) dezelfde method aanroept om een waarde te setten. Laten we even uitgaan van een postcode of telefoonnummer. In het begin neem ik alles aan, 1234 ab, 1234 AB, 1234ab en 1234AB en ik bedenk me later dat ik toch liever alles eenduidig wil hebben. Dan kan ik dat in deze method aanpassen. Anders moet ik dit in alle andere scriptjes gaan aanpassen waar het als volgt wordt geset: $Obj->postcode = $_POST['postcode']; Ervanuitgaand dat ik niet in de method wil gaan if-elsen waarin ik de data gebruik (kunnen er ook meerdere zijn).