Inladen van methods in constructor, goed idee?
Dit systeem werkt met een Rights-class met o.a de method getRights(). Deze roep ik nu aan in de constructor zodat automatisch de rechten-array geladen wordt bij het instancieren van class met:
Code (php)
1
2
3
2
3
<?php
$rights = new Rights();
?>
$rights = new Rights();
?>
Nu vroeg ik me af of dit een goed idee is, en of het misschien toch beter is om gewoon de getRights() aan te roepen nadat de class geinstancieerd wordt i.p.v. de constructor te gebruiken. Dus:
Code (php)
1
2
3
4
2
3
4
<?php
$rights = new Rights();
$rights->getRights();
?>
$rights = new Rights();
$rights->getRights();
?>
Mijn class is (versimpeld) als volgt:
Code (php)
Iemand een goed advies hierover?
Zoals ik je voorbeeld nu bekijk, zou ik het ophalen van de rechten in de constructor afhandelen en intern opslaan.
Ik zou het volgende dus aanraden:
Code (php)
1
2
3
4
2
3
4
<?php
$rights = new Rights();
$rights->getRights();
?>
$rights = new Rights();
$rights->getRights();
?>
Is het niet meer logisch om een array te hebben met Right classes erin?
Want ik ziet geen toegevoegde waarde van een Rights class.
De class die de rechten ophaalt zou een array met rechten moeten teruggeven.
En zou nadenken of Right(s) wel goede benamingen zijn.
@Remco: Er zijn in werkelijkheid nog meer methods binnen de rights-class, zoals checkRights(). Dan is het toch vrij normaal om dit op te slaan in een Rights-class?
Als ik zoiets doe:
Code (php)
1
2
3
2
3
<?php
$rights = new Rights();
?>
$rights = new Rights();
?>
Dan zou ik in eerste instantie verwachten dat $rights een referentie is naar het object (en dit is ook wat je terugkrijgt).
Als je het mij vraagt mis je een stap, dit:
Code (php)
1
2
3
2
3
<?php
// maak hier de array met de rechten die je returned
?>
// maak hier de array met de rechten die je returned
?>
Zou je in een aparte methode kunnen onderbrengen. Die je al dan niet bij creatie aanroept, of apart.
Bijvoorbeeld dus als volgt:
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 Rights
{
protected $rights; // gebruik bij voorkeur protected i.p.v. private zodat je classes kunt extenden
public function __construct() {
$this->rights = false; // init op een "ongeinitialiseerde" waarde, of je laat deze NULL
}
public function init($args) {
$this->rights = array(); // voor als $args verder niets oplevert
// verdere initialisatie op grond van $args
return $this; // voor method chaining
}
public function get() {
if ($this->rights === false) { // of NULL, als je deze bij creatie niet initialiseert op false
throw new Exception('rights was not properly initialized');
}
return $this->rights;
}
}
$r = new Rights();
$rights = $r->init(array('hoi' => 'pipeloi'))->get();
var_dump($rights);
?>
class Rights
{
protected $rights; // gebruik bij voorkeur protected i.p.v. private zodat je classes kunt extenden
public function __construct() {
$this->rights = false; // init op een "ongeinitialiseerde" waarde, of je laat deze NULL
}
public function init($args) {
$this->rights = array(); // voor als $args verder niets oplevert
// verdere initialisatie op grond van $args
return $this; // voor method chaining
}
public function get() {
if ($this->rights === false) { // of NULL, als je deze bij creatie niet initialiseert op false
throw new Exception('rights was not properly initialized');
}
return $this->rights;
}
}
$r = new Rights();
$rights = $r->init(array('hoi' => 'pipeloi'))->get();
var_dump($rights);
?>
EDIT: bij initialisatie-bij-creatie geef je $args door aan je constructor, en zou je de init() methode protected kunnen maken omdat je deze toch enkel "intern" aanroept.
Gewijzigd op 23/02/2016 15:56:09 door Thomas van den Heuvel
Dus als jouw Rights class de 'manager' is om dingen te checken dan zou ik via de constructor een class meegeven welke de rechten ophaalt en in je getRights iets doen van $this->rightsLoader->getRights();. En die rightsloader haalt dan de dingen op. Het is ook maar hoe ver je wilt gaan hoor.
Maar om basaal antwoord te geven:
- Dingen die je altijd nodig hebt roep je direct in de constructor op
- Andere dingen doe je pas als de methode wordt aangeroepen.
Maar (invulling) ik zou wel gaan kijken of meer 'separation of concerns' kan toepassen.
Ik ga zeker eens kijken naar Thomas zijn idee, en de constructor niet standaard de rechtenstructuur laten ophalen. Volgens mij gebeurt dit namelijk steeds opnieuw als je een method aanroept.
a. Wil je daadwerkelijk rechten doorgeven?
Hier, alsjeblieft, heb je mijn toegangspas.
b. Of wil je rechten controleren?
Heb jij een roze of blauw toegangspasje met de juiste geldigheidsdatum?
In het tweede geval is het logischer een methode hasRights() toe te voegen die true of false retourneert. Zo ontkoppel je de boel. Hoe die rechten verder werken en waar ze vandaan komen is voor de controle niet relevant.
Als je een class Rights hebt die alle rechten cadeau doet met een getRights(), dan is het eigenlijk slechts een container. En dat brengt ons dan bij punt twee: het is slechts een datamodel, waaraan je voor het gebruik van de data misschien nog één of meer controllers moet toevoegen. Bijvoorbeeld een RightsManager of een meer dedicated RightsReader, RightsWriter, RightsRevoker, enzovoort, enzovoort.
Ward van der Put op 23/02/2016 16:52:27:
In het tweede geval is het logischer een methode hasRights() toe te voegen die true of false retourneert. Zo ontkoppel je de boel. Hoe die rechten verder werken en waar
Mja en in dat geval kun je je ook afvragen of je wel objecten zou moeten bakken omdat deze verder toch niet echt een state hebben, je zou in dat geval ook kunnen volstaan met een static helper class?
Code (php)
1
2
3
4
5
2
3
4
5
<?php
if (Authorization::isAllowed($grantedRights, $requiredRights)) {
// do stuff
}
?>
if (Authorization::isAllowed($grantedRights, $requiredRights)) {
// do stuff
}
?>
Thomas van den Heuvel op 23/02/2016 16:59:53:
Mja en in dat geval kun je je ook afvragen of je wel objecten zou moeten bakken omdat deze verder toch niet echt een state hebben, je zou in dat geval ook kunnen volstaan met een static helper class?
Ward van der Put op 23/02/2016 16:52:27:
In het tweede geval is het logischer een methode hasRights() toe te voegen die true of false retourneert. Zo ontkoppel je de boel. Hoe die rechten verder werken en waar
Mja en in dat geval kun je je ook afvragen of je wel objecten zou moeten bakken omdat deze verder toch niet echt een state hebben, je zou in dat geval ook kunnen volstaan met een static helper class?
Code (php)
1
2
3
4
5
2
3
4
5
<?php
if (Authorization::isAllowed($grantedRights, $requiredRights)) {
// do stuff
}
?>
if (Authorization::isAllowed($grantedRights, $requiredRights)) {
// do stuff
}
?>
Het gaat een beetje off-topic, maar waarom zou een User niet zelf mogen weten of het voldoende rechten heeft? $user->isAllowed($requiredRights)
Thomas van den Heuvel op 23/02/2016 16:59:53:
Dat kan ook, maar dan mis je misschien nog een onderdeel ... een class Rights die definieert hoe een collectie rechten eruit ziet. ;)Mja en in dat geval kun je je ook afvragen of je wel objecten zou moeten bakken omdat deze verder toch niet echt een state hebben, je zou in dat geval ook kunnen volstaan met een static helper class?
Code (php)
1
2
3
4
5
2
3
4
5
<?php
if (Authorization::isAllowed($grantedRights, $requiredRights)) {
// do stuff
}
?>
if (Authorization::isAllowed($grantedRights, $requiredRights)) {
// do stuff
}
?>
Remco van Bers op 23/02/2016 17:10:44:
Het gaat een beetje off-topic, maar waarom zou een User niet zelf mogen weten of het voldoende rechten heeft? $user->isAllowed($requiredRights)
Omdat het niet altijd/enkel van de (huidige) user zelf af hoeft te hangen of iets toegankelijk is.
Daarnaast, hoe zou een implementatie van het bovenstaande eruitzien? Zoiets?
Code (php)
Is dan niet meer dan een (weliswaar handige, maar nog steeds een) shorthand/alias.
Toevoeging op 23/02/2016 18:11:53:
Ward van der Put op 23/02/2016 17:52:42:
Dat kan ook, maar dan mis je misschien nog een onderdeel ... een class Rights die definieert hoe een collectie rechten eruit ziet. ;)
Fair enough, daar kun je afspraken over maken (en dat hoeft niet per se in een class te gebeuren).
Gewijzigd op 23/02/2016 18:13:35 door Thomas van den Heuvel