Doe ik OOP goed?
Dit soort classes maken is natuurlijk altijd goed om te leren hoe je oop moet programmeren. Hopelijk heb je wat aan de volgende opmerking(en):
De scope van je functies zijn te groot. Je zou per functie slechts één ding moeten doen. Klein voorbeeld van een aanroep(uit de duim gezogen) waarbij elke functie WEL 1 ding doet:
$kareltje = $database->select("user")->whereEquals('id', 5)->whereEquals("nog een vergelijking", "...")->order("id", "DESC")->firstOrDefault();
Ik wil je code niet afzeiken(het doet naar verluid wat het moet doen, en daar gaat het om), maar ik ga ervan uit dat je jouw programmeerkunsten wilt verbeteren.
Als je als doel stelt dat een functie écht maar 1 ding doet, krijg je code die gemakkelijk onderhouden kan worden. Om functionaliteit toe te voegen kan je namelijk een functie toevoegen ipv een functie aan te passen. Het aanpassen van een functie is bug- gevoelig, vaak zelfs onmogelijk zonder er een bak met tijd overheen te gooien.
Verder kan het handig zijn om "magical strings" in constanten op te slaan. "Where clause is empty for update method" komt bijvoorbeeld 2x voor in je code. Dubbele code moet je altijd proberen te voorkomen. dit kan je oplossen door een constant toe te voegen aan de class met de desbetreffende text. Persoonlijk sla ik constants altijd op in een aparte class. Dit doe je ten slotte ook met taalspecifieke onderdelen.
Hopelijk heb je hier al wat aan.
Nog wat tips:
https://laravel-news.com/clean-code-php-guide
Gewijzigd op 27/12/2017 20:27:59 door phpnuke r
https://github.com/tschoffelen/db.php/blob/master/Database.php
Het is een beetje raar om hier andermans code te laten nakijken.
- Rob - op 26/12/2017 14:28:39:
En hoe moet ik dan zorgen dat de User class de database connectie kan gebruiken? Moet ik daarvoor global $conn gebruiken, maar global is volgens vele mensen niet veilig..
Nee geen Global maar injections.
DatabaseInterface.php
Code (php)
1
2
3
4
5
6
7
8
2
3
4
5
6
7
8
<?php
interface DatabaseInterface
{
// een lijst met verplichte functies die een Database class VERPLICHT MOET hebben
// google eens op 'php interface'
{
?>
interface DatabaseInterface
{
// een lijst met verplichte functies die een Database class VERPLICHT MOET hebben
// google eens op 'php interface'
{
?>
Database.php
Code (php)
1
2
3
4
5
6
2
3
4
5
6
<?php
class Database implements DatabaseInterface
{
// 'De' database class. De koppeling tussen database en jouw applicatie
}
?>
class Database implements DatabaseInterface
{
// 'De' database class. De koppeling tussen database en jouw applicatie
}
?>
User.php
Code (php)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
<?php
// Deze class slaat de gegevens van één user op in het geheugen. een soort OOP associatieve array dus
class User {
private username;
// etc
public function setUsername($username) {
$this->username = $username;
return $this; // google op 'php method chaining'
}
public function getUsername() {
return $this->username;
}
// etc
}
?>
// Deze class slaat de gegevens van één user op in het geheugen. een soort OOP associatieve array dus
class User {
private username;
// etc
public function setUsername($username) {
$this->username = $username;
return $this; // google op 'php method chaining'
}
public function getUsername() {
return $this->username;
}
// etc
}
?>
Repository.php
Code (php)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
2
3
4
5
6
7
8
9
10
11
12
13
14
15
<?php
class Repository
{
private $db;
public __contstruct(DatabaseInterface $database) {
$this->db = $database;
}
// deze class is een base class voor UserRepository maar later ook voor OrderRepository, CustomerRepository en weet ik veel (bedenk het maar)
// hier schrijf je dus methods die door de child classes gebruikt kunnen worden
}
?>
class Repository
{
private $db;
public __contstruct(DatabaseInterface $database) {
$this->db = $database;
}
// deze class is een base class voor UserRepository maar later ook voor OrderRepository, CustomerRepository en weet ik veel (bedenk het maar)
// hier schrijf je dus methods die door de child classes gebruikt kunnen worden
}
?>
UserRepository.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
<?php
class UserRepository extends Repository
{
public __contstruct(DatabaseInterface $database) { // <-- DatabaseINTERFACE en niet Database. zie commentaar aan het einde
parent::__construct($database);
}
// hier ga je je queries met betrekking tot de user tabel schrijven
public function findUserByUsername($username) {
$result = $db->query("SELECT * FROM users WHERE username='$username'");
if($result) {
$user = new User();
$user->fromArray($db->fetch_assoc($result));
return $user;
}
}
// meer 'query' functies
}
?>
class UserRepository extends Repository
{
public __contstruct(DatabaseInterface $database) { // <-- DatabaseINTERFACE en niet Database. zie commentaar aan het einde
parent::__construct($database);
}
// hier ga je je queries met betrekking tot de user tabel schrijven
public function findUserByUsername($username) {
$result = $db->query("SELECT * FROM users WHERE username='$username'");
if($result) {
$user = new User();
$user->fromArray($db->fetch_assoc($result));
return $user;
}
}
// meer 'query' functies
}
?>
Waarom een database interface?
Waarschijnlijk wil je nu een mysql database gaan gebruiken. Maar wat nu als je later je code met een MongoDatabase wilt laten werken? Of met een MySqLite database? Dan kun je een nieuwe Database class maken die ook DatabaseInterface implementeert en dan weet je dat deze class met AL je code gewoon werkt,
Gewijzigd op 28/12/2017 00:26:11 door Frank Nietbelangrijk
- Rob - op 27/12/2017 19:27:55:
Hey,
Is dit een goede database class?:
<knip>
Is dit een goede database class?:
<knip>
Nee, niet echt.
Hierover verschillen de meningen, maar final en private verdienen in principe niet de voorkeur omdat dit niet het uitbreidbaarheidsbeginsel van object-georiënteerde code volgt.
Daarnaast introduceert het nogal wat abstractie, de vraag is of je hier iets aan hebt wanneer je (MySQL-)specifieke queries aan het opstellen bent. Persoonlijk vind ik het veel fijner wanneer je zelf de volledige controle heb over hoe een query er uit ziet. Ook is dat vaak handiger wanneer je queries moet debuggen. Deze class regelt ook niets omtrent transacties :/.
Maar het meest funeste is nog dat deze database-class niet expliciet een character encoding instelt bij het maken van je connectie. Escaping-functionaliteit is (mede) afhankelijk van een juiste character encoding. Dit had ik eigenlijk al aangegeven in een vorige reactie. Hierin staat tevens een link naar een andere implementatie. Ik zeg niet dat deze beter is (dit is mijn eigen wiel) maar heb je hier uberhaupt naar gekeken?
En inderdaad, wanneer je met OOP begint te werken is het vooral belangrijk dat je niet direct teveel hooi op je vork neemt.
EDIT: en dat is misschien een aparte discussie waard: in hoeverre is (een ver doorgevoerde mate van) abstractie nog van toegevoegde waarde. Je kunt wel allerlei tussenlagen inbouwen om in het midden te laten wat code uiteindelijk moet doen, maar uiteindelijk zal het toch iets moeten doen. Door abstractie wordt code ook abstracter :). Dit maakt het moeilijker leesbaar, moeilijker onderhoudbaar, dit kost ook tijd en dus geld. En mogelijk benut je nooit echt de geïntroduceerde flexibiliteit en in dat geval is al die code overbodige fluf.
EDIT #2: toegepast op @Frank zijn voorbeeld, en dit is niet bedoeld om zijn reactie af te kraken want deze code is ongetwijfeld opgesteld volgens de regels der kunst, maar hoe vaak zal er in de levensloop van een applicatie (vrij) geschakeld worden tussen verschillende database-typen? Het is denk ik vooral belangrijk om praktisch te blijven. OOP is een middel, geen doel, dus om in OOP te programmeren "enkel voor de vorm" lijkt mij niet de goede insteek. Je moet een reden hebben om dingen op een OOP-manier aan te pakken. PHP is een hybride taal, dus het is mogelijk om hybride (OOP/procedureel) te programmeren.
Daarintegen, er is nog steeds iets voor te zeggen om een wrapper om MySQL(i)-functies of -methoden te schrijven. Dit om de volgende reden: er zou van alles in MySQL-land kunnen veranderen en dat zou kunnen betekenen dat je dan de manier waarop je met je database communiceert om moet gooien. Wanneer deze omgang met de database rechtstreeks plaatsvondt zou dit inhouden dat alle aanroepen hardcoded in code zouden zitten. Dit zou dan ook kunnen inhouden dat je dus alle of een heleboel instanties van deze aanroepen moet aanpassen als hier iets in is veranderd. Ook al lijken wrappers redelijk loos (en je zult zelf moeten bepalen of (en zorgen dat) er een zekere meerwaarde bij gebruik is), deze zorgen nog steeds voor een zekere mate van abstractie - ze stellen je in staat om indirect met je database te communiceren. Mocht er bij gebruik van wrappers iets veranderen in MySQL, dan hoef je -idealiter uiteraard- maar op één plek code aan te passen.
Gewijzigd op 28/12/2017 14:37:13 door Thomas van den Heuvel
Thomas van den Heuvel op 28/12/2017 14:13:49:
Het is denk ik vooral belangrijk om praktisch te blijven. OOP is een middel, geen doel, dus om in OOP te programmeren "enkel voor de vorm" lijkt mij niet de goede insteek. Je moet een reden hebben om dingen op een OOP-manier aan te pakken.
Mooi gezegd, en helemaal waar.
+1
Abstractie is natuurlijk zeker niet het doel van programmeren. Flexibiliteit daar in tegen wel. Hoe flexibeler je code hoe vaker je je code kunt hergebruiken.
>> hoe vaak zal er in de levensloop van een applicatie (vrij) geschakeld worden tussen verschillende database-typen?
(Bijna) niet. Maar wanneer je je code wilt hergebruiken voor een andere klant of een ander project dan is de kans wel aanwezig dat je met een andere database-type te maken krijgt.
Er zijn met behulp van Composer veel goede standaard libraries te vinden. Deze kunnen dan later (ook wanneer je project al lang afgerond is) ge-update worden. En dat kan zeker wel noodzakelijk zijn. Zo zijn er in het verleden PHP functies als verouderd of onveilig bestempeld. (deprecated). Een recent voorbeeld zijn de mysql_ functies die uit PHP versie 7 verbannen zijn. Wie weet komt er in de toekomst een vervanger voor mysqli of voor de password hashing functies. Stel dat dit gebeurt en de server waar jouw code op draait wordt voorzien van de nieuwste PHP versie zonder de oude deprecated functies dan ben je een blij persoon als enkel maar de updates hoeft op te halen van de libraries die je gebruikt.
Als je heel erg strict wilt gaan worden dan moet je eigenlijk in je bovenste laag van je code helemaal geen standaard PHP functie meer aanroepen. In plaats daarvan roep je een method in een van je libraries aan. Mochten de makers van PHP een functie weglaten of wijzigen of iets dergelijks dan hoef je alleen maar een library aan te passen en/of te updaten.
Frank Nietbelangrijk op 05/01/2018 18:17:19:
Abstractie is natuurlijk zeker niet het doel van programmeren. Flexibiliteit daar in tegen wel. Hoe flexibeler je code hoe vaker je je code kunt hergebruiken.
Maar iets dat heel abstract is, is juist nauwelijks concreet en daarmee dus universeler inzetbaar.
Met andere woorden: abstractie en flexibiliteit sluiten elkaar niet uit, maar versterken elkaar juist.
(Het praktische probleem is meer dat mensen het erg moeilijk vinden om de vertaalslag te maken van een universele abstractie naar hun onderhanden, concreet probleem. Maar dat is dan vooral hun probleem eigenlijk.)