Ik programmeer nu toch al een tijdje, en heb al veel de tip gekregen om eens te beginnen met OOP. Ik had er wat over gelezen op internet en het leek mij allemaal iets te moeilijk. Ik heb mezelf nu toch eens kunnen overhalen om eens een scriptje in classes te schrijven.
Ik heb dus een mini-telefoonboekje geschreven en natuurlijk had ik graag tips gehad. Ik ken dus zo goed als niks van OOP, dus graag tips en opmerkingen.
add_record moet in telefoonboek, je kan niet een record toevoegen aan een vermelding zelf. Wel aan een telefoonboek. En add_record geef je dan een instantie van Vermelding mee als argument, want die wil je immers toevoegen.
Obj-C is eigenlijk een hele handige taal om het idee achter OOP te leren zo gezien. In Obj-C stuur je namelijk 'berichten' naar objecten. Voorbeeldje:
[telefoonboek addRecord:mijnVermelding];
waarin telefoonboek een instantie is van Telefoonboek, en mijnVermelding een instantie van Vermelding. Je zegt als het ware tegen het telefoonboek dat hij die vermelding moet opnemen. Logischer kan het volgens mij niet.
Ik heb het nogmaals geprobeerd, hopelijk nu beter...
phonebook.class.php
<?php
class Phonebook
{
public function __construct()
{
// Niks?
}
public function add_record($obj)
{
$query = "INSERT INTO persons (name, number) VALUES('".mysql_real_escape_string($obj->name)."', '".mysql_real_escape_string($obj->number)."')";
$result = mysql_query($query) or die(mysql_error());
if($result)
{
return true;
}
else
{
return false;
}
}
public function get_all_records()
{
$query = "SELECT id, name, number FROM persons";
$result = mysql_query($query) or die(mysql_error());
while($row = mysql_fetch_assoc($result))
{
$record = new Phonerecord;
$record->set_id($row['id']);
$record->set_name($row['name']);
$record->set_number($row['number']);
$record->get_record();
$records[] = $record;
}
return $records;
}
}
?>
phonerecord.class.php
<?php
class Phonerecord
{
public $id;
public $name;
public $number;
public function __construct()
{
//
}
public function set_id($id)
{
$this->id = $id;
}
public function set_name($name)
{
$this->name = $name;
}
public function set_number($number)
{
$this->number = $number;
}
public function get_record()
{
$query = "SELECT id, name, number FROM persons WHERE id = '".$this->id."'";
$result = mysql_query($query) or die(mysql_error());
if(!$result)
{
return false;
}
else
{
$row = mysql_fetch_assoc($result);
$this->id = $row['id'];
$this->name = $row['name'];
$this->number = $row['number'];
return true;
}
}
}
?>
index.php
<?php
include "mysql.class.php";
include "phonebook.class.php";
include "phonerecord.class.php";
$db = new MySQL('localhost', '***', '***');
$db->select_db('***');
if($_SERVER['REQUEST_METHOD'] == "POST")
{
if(!empty($_POST['name']) && !empty($_POST['number']))
{
$phonebook = new Phonebook;
$obj = new Phonerecord;
$obj->set_name(htmlspecialchars($_POST['name']));
$obj->set_number(htmlspecialchars($_POST['number']));
if($phonebook->add_record($obj))
{
echo 'Your record has been added.<br />';
}
else
{
echo 'Something went wrong while adding the record.<br />';
}
}
else
{
echo 'You need to fill in all the fields.<br />';
}
}
echo '
<table border="1">
<tr>
<td width="60">Name</td><td width="110">Number</td><td width="30">Detail</td>
</tr>';
foreach(Phonebook::get_all_records() as $record)
{
echo '
<tr>
<td>'.$record->name.'</td><td>'.$record->number.'</td><td align="center"><a href="?id='.$record->id.'">X</a></td>
</td>
</tr>';
}
echo '</table><br />';
?>
<form action="" method="post">
Name: <input name="name" type="text" /><br />
Number: <input name="number" type="text" /><br /><br />
<input name="Submit" type="submit" value="Submit" id="Submit" />
</form>
Nu moet het gewoon goed :S Ik denk dat het redelijk goed gelukt is, behalve bij get_record. Ik had de keuze om zonder argument of met te werken. Ik heb zonder gewerkt omdat get_record() gewoon de gegevens van het huidige object moet hebben. Anders kon ik ook een id als argument meegeven, maar dan kan hij de gegevens van allemaal ophalen. Ik weet niet welke best is, maar ik nam deze omdat ik denk datie enkel de gegevens van zichzelf nodig heeft.
De get_record method van PhoneRecord is eigenlijk niet nodig, aangezien je de data al ophaalt in Phonebook. Je voert nu voor ieder record dat je ophaalt een extra - nutteloze - query uit.
Als je de mogelijkheid wilt hebben om individuele records op te halen, zou ik de get_record method verplaatsen naar Phonebook, en hem inderdaad zoals je zelf al zegt een id als argument laten slikken. Bovendien is het voordelig als je deze method naar Phonebook verplaatst, aangezien je dan alle opslag-gerelateerde functies alleen binnen Phonebook hebt, en Phonerecord er niet meer gevoelig voor is (en in theorie, mocht je de database ook via klassen willen gaan doen, niet meer afhankelijk is van een instantie van de database)
Verder zie ik dat je in Phonerecord wel accessors (de get_ en set_ method) hebt, maar je eigenschappen ook public. Accessors zijn er eigenlijk voor bedoeld dat de werking binnenin het object niet naar buiten komt. Dus probeer de eigenschappen zoveel mogelijk protected te houden, en het object van buitenaf alleen via de accessors aan te spreken. Ook binnen phonebook.
Je gebruikt bij je queries 'or die()', terwijl je daarna ook nog controleert of het resultaat wel bestaat (en indien niet, false teruggeeft) Het is een good practice om je klassen zo verlegen mogelijk te maken. Laat ze geen uitvoer echo'en, en zeker geen scripts halverwege afkappen wanneer een query mislukt. Hiervoor kun je sinds PHP 5 heel goed Exceptions gebruiken. Op die manier kan je haperende klassen binnen je script opvangen zonder dat de rest daar last van hoeft te hebben.
In index.php zie ik dat je htmlspecialchars over de gegevens in Phonerecord haalt. Dat heeft als gevolg dat je al opgemaakte data in je database zet. Voor HTML alleen zou dat niet snel problemen opleveren. Maar stel dat je de data ook wilt gaan gebruiken in emailtjes, of in plain text-documenten. Dan is de data die je in je database hebt al 'bevuilt' met HTML. Het is een goed idee om de data zo rauw mogelijk in je database op te slaan, en het escapen van speciale karakters pas te doen wanneer de data wordt weergegeven.
Ik maak altijd een altijd eerste een algemene classe waarin in mijn sql verbinding opneem. Ook maak ik een functie waarin in mijn errors opsla, een functie voor het tellen van errors en het weergeven van errors.
Zo kan ik ook netjes de form validatie in mijn classe uitvoeren en krijg ik nette errors op mijn scherm. Alles afvangen is natuurlijk een must. Een debug variabele geeft precies de hoeveelheid aan informatie die de error moet bevatten aan. Ik vind het heel prettig werken, aangezien ik elke classe dus extend met deze standaard classe, en ik mijn output, (bijvoorbeeld jou index.php) enorm kan inkorten. Dit werkt extra snel en makkelijk wat betreft de opmaak.
Wat ik nu wel heb is dat ik bij een get_record een instance van een phonerecord moet meegeven. Is dit wel de juiste manier van werken?
Het is niet echt nodig. Ik zou get_record de instantie laten aanmaken. Dat scheelt je weer herhalend werk en het is eigenlijk logischer. In je huidige opzet is het meer een fill_record dan een get_record.
Trouwens, je kan nu bijvoorbeeld add_record de klasse van het argument laten afdwingen:
public function add_record(Phonerecord $record) {
Dat maakt je code weer wat leesbaarder en foutjes komen een heel klein beetje sneller aan het licht.
Ow, ik bedenk me net, je kan ook [php]mysql_fetch_object[/php] gebruiken. Die accepteert als 2e argument een klasse. Dat scheelt je weer 3 setters aanroepen per object:
Sterker nog, nu zou je de set_id-accessor kunnen weglaten. Een id is immers ook niet aan te passen, die is uniek en permanent per record, dus een setter daarvoor hebben is eigenlijk niet logisch.
Het is niet echt nodig. Ik zou get_record de instantie laten aanmaken. Dat scheelt je weer herhalend werk en het is eigenlijk logischer. In je huidige opzet is het meer een fill_record dan een get_record.
Ik liet de functie de instantie aanmaken: $record = new Phonerecord; En dan returnde ik op het einde $record. Maar toen was ik verplicht in het script $record te gebruiken als object inplaats van $rec.
Op zich geen probleem toch? Ik kan me niet voorstellen dat je bewerkingen op een Phonerecord-instantie wilt doen voordat je hem vult met de gegevens die erin moeten komen.
<?php
$rec = $phonebook->get_record(24);
$rec->set_name('Klaas de 2e');
$phonebook->save_record($rec);
?>
Dat lijkt mij de meest logische en gemakkelijke volgorde.
Om methodes zoals get_data, set_name en set_data te voorkomen zou je ook eens kunnen kijken naar de magic methods in PHP5. Staat op deze site een prima tutorial over.
Ghehe, heb ik die geschreven? Kan ik me niets van herinneren :P
Anyway, magic methods zijn inderdaad ook een oplossing om je getters & setters te verbergen. Of je ze wilt gebruiken is puur persoonlijk. Ze bieden verder geen voordelen in de zin van snelheid of gebruikersgemak. Zelf geef ik tegenwoordig meer de voorkeur aan ouderwetse methods, omdat je daar al klassen kan afdwingen in de definitie van de argumenten van de method, en de methods mooi naar voren komen in de documentatie en de andere hulpmiddelen zoals Eclipse. Maar een jaar geleden zou ik nog __get & __set gebruikt hebben. Tegenwoordig gebruik ik die alleen nog maar om trucjes als 'lazy loading' en 'chainability' in te kunnen bouwen.
