best practice: isset / !isset

Overzicht Reageren

Sponsored by: Vacatures door Monsterboard

Ozzie PHP

Ozzie PHP

15/04/2014 00:33:42
Quote Anchor link
Ola,

Een vraagje voor de (afgestudeerde) programmeurs. Als ik een User class heb en ik wil iemands volledige naam weten, dan moet deze één keer worden gegenereerd.

Nu vraag ik me af. Ik kan in de getFullName() method controleren:

a) of de volledige naam al bestaat, zo ja -> teruggeven, zo nee -> genereren en teruggeven

óf

b) of de naam niet bestaat, zo niet -> genereren, vervolgens de volledige naam teruggeven

Zelf kies ik voor aanpak b) maar ik vraag me af of de programmeurs onder ons dat ook doen.

Even de voorbeeldjes uitgewerkt in vereenvoudigde code:

a)

Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
<?php
class User {

  private $first_name;
  private $full_name;
  private $last_name;

  public function getFullName() {
    if (isset($this->full_name)) return $this->full_name;
    return $this->generateFullName();
  }


  private function generateFullName() {
    $full_name       = $this->first_name . ' ' . $last_name;
    $this->full_name = $full_name;
    return $full_name;
  }


?>

b)

Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
<?php
class User {

  private $first_name;
  private $full_name;
  private $last_name;

  public function getFullName() {
    if (!isset($this->full_name)) $this->generateFullName();
    return $this->full_name;
  }


  private function generateFullName() {
    $this->full_name = $this->first_name . ' ' . $last_name;
  }


?>

Zoals ik zei zou ik zelf optie b) gebruiken, maar is dat ook de "juiste" aanpak?
 
PHP hulp

PHP hulp

28/12/2024 22:12:24
 
Willem vp

Willem vp

15/04/2014 02:04:53
Quote Anchor link
Zelf gebruik ik meestal optie a, maar dat komt misschien omdat ik meer in Perl werk, waar je shortcuts kunt uitvoeren als:

return $this->full_name || $this->generateFullName();

Aan de andere kant heeft generateFullName() in fragment a als side effect dat er niet alleen een waarde wordt teruggegeven, maar ook een class property wordt gewijzigd. In dat geval zou fragment b wat netter zijn.
 
Ger van Steenderen
Tutorial mod

Ger van Steenderen

15/04/2014 06:19:16
Quote Anchor link
Ik kies voor optie c:
Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
4
5
6
7
8
9
10
11
12
13
14
<?php

class User {

    private $firstName;
    private $lastName;

    public function getFullName() {
        return $this->firstName . ' ' . $this->lastName;
    }

}


?>
Gewijzigd op 15/04/2014 06:21:56 door Ger van Steenderen
 
Ozzie PHP

Ozzie PHP

15/04/2014 09:19:18
Quote Anchor link
@Willem:

Ik zou in de lijn van jouw voorbeeld ook de ternary operator kunnen gebruiken!

Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
4
5
<?php
  public function getFullName() {
    return isset($this->full_name) ? $this->full_name : $this->generateFullName();    
  }

?>

Is dat dan een nettere/efficiëntere manier?

@Ger:

Dat kan ook. In de praktijk zou er ook nog een tusenvoegsel kunnen zijn en is de method dus iets complexer. Dan vind ik het handiger om éénmaal de naam te genereren en daarna de gegenereerde naam terug te geven.
 
Ger van Steenderen
Tutorial mod

Ger van Steenderen

15/04/2014 09:54:22
Quote Anchor link
Ozzie's way:
Code (php)
PHP script in nieuw venster Selecteer het PHP script
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
<?php

class OrderItem {

    private $price;
    private $amount;
    private $total;

    public function getTotal {
         return isset($this->total) ? $this->total : $this->calculateTotal();
    }


    public function setPrice($price) {
        $this->price = $price;
        $this->calculateTotal();
    }


    public function setAmount($amount) {
        $this->amount = $amount;
        $this->calculateTotal();
    }


    private function calculateTotal() {
        $this->total = $this->price * $this->amount;
        return $this->total;
    }

}

?>

Leg jij nu maar eens uit wat daar handig aan is.
Gewijzigd op 15/04/2014 09:58:58 door Ger van Steenderen
 
Ozzie PHP

Ozzie PHP

15/04/2014 10:00:10
Quote Anchor link
Dat hangt van de situatie af.

Waarschijnlijk bedoel jij te zeggen dat in jouw voorbeeld de "prijs" en het "aantal" kunnen veranderen, ofwel het is dynamische data. En ja, dan moet je het totaalbedrag dus real-time berekenen. Dat ben ik volledig met je eens.

Echter, zover ik weet heet een user niet het ene moment Ger van Steenderen en even later Ger Pietersen. Anders gezegd, een naam is een vast en onveranderlijk gegeven. Waarom zou je die iedere keer gaan "berekenen"?
 
Willem vp

Willem vp

15/04/2014 10:03:07
Quote Anchor link
Ternary zou inderdaad ook kunnen, maar ook hier geldt dat je dan nog steeds te maken hebt met het side-effect van generateFullName().

Ik zou het denk ik mooier vinden als getFullName() alleen een return $this->full_name doet en dat bij elke wijziging van voornaam, achternaam of tussenvoegsel de fullname automatisch wordt gegenereerd.

Als je daarnaast in de constructor de initiële waarde van fullname op '' zet (lege string) is dat hele gedoe met isset() ook niet nodig.

Toevoeging op 15/04/2014 10:06:29:

Ozzie PHP op 15/04/2014 10:00:10:
Waarom zou je die iedere keer gaan "berekenen"?

Dat is ook precies mijn punt. ;-) Ik heb er een beetje een hekel aan om kostbare CPU-cycles te verspillen aan het steeds herberekenen van dingen die redelijk statisch zijn.

Wel is het zo dat ik vind dat je in een getter geen berekeningsfuncties moet afvuren. In het voorbeeld van Ger zou getTotal() bij mij dus alleen een totaal teruggeven en niet eventueel nog eerst het totaal berekenen.
Gewijzigd op 15/04/2014 10:09:01 door Willem vp
 
Ger van Steenderen
Tutorial mod

Ger van Steenderen

15/04/2014 10:06:34
Quote Anchor link
Als die naam dan zo'n vast gegeven is, dan stel je hem in de constructer vast.
Je bent niet functionele functies aan het creeëren.
Gewijzigd op 15/04/2014 10:07:24 door Ger van Steenderen
 
Ozzie PHP

Ozzie PHP

15/04/2014 10:12:31
Quote Anchor link
@Willem:

>> Ternary zou inderdaad ook kunnen, maar ook hier geldt dat je dan nog steeds te maken hebt met het side-effect van generateFullName().

En met side-effect bedoel je dan dat 1 method zowel iets returnt als een property set? En waarom "mag" dat niet?

>> Ik zou het denk ik mooier vinden als getFullName() alleen een return $this->full_name doet en dat bij elke wijziging van voornaam, achternaam of tussenvoegsel de fullname automatisch wordt gegenereerd.

Oké. Alleen dat is ook raar, want dan set ik de voornaam Willem, en dat gaat ie de full_name genereren. Vervolgens set ik de achternaam vp en dan gaat ie weer genereren.

@Ger:

>> Je bent niet functionele functies aan het creeëren.

Dat vind ik een wat overdreven opmerking...

>> Als die naam dan zo'n vast gegeven is, dan stel je hem in de constructer vast.

Als ik de gegevens wil inladen via een datamapper, dan moet ik die setten via public setters. Dan ben ik de constructor dus al gepasseer. Anders had ik het inderdaad in de constructor kunnen doen, maar dat gaat dus niet.
 
Ger van Steenderen
Tutorial mod

Ger van Steenderen

15/04/2014 10:24:35
Quote Anchor link
Als je een eigenschap gaat samenstellen uit andere eigenschappen die public setters hebben, zal je hem telkens moeten aanpassen als één van die eigenschappen geset wordt.

We zijn in jouw voorbeeld niet met complexe algoritmes bezig, maar gewoon een paar strings aan elkaar aan het plakken.
 
Ozzie PHP

Ozzie PHP

15/04/2014 10:31:19
Quote Anchor link
>> We zijn in jouw voorbeeld niet met complexe algoritmes bezig, maar gewoon een paar strings aan elkaar aan het plakken.

Ja, maar daar gaat het niet om. Het gaat mij met name om "de juiste aanpak".

>> Als je een eigenschap gaat samenstellen uit andere eigenschappen die public setters hebben, zal je hem telkens moeten aanpassen als één van die eigenschappen geset wordt.

Tja, daar zit iets in.
 
Willem vp

Willem vp

15/04/2014 14:20:16
Quote Anchor link
Ozzie PHP op 15/04/2014 10:12:31:
Oké. Alleen dat is ook raar, want dan set ik de voornaam Willem, en dat gaat ie de full_name genereren. Vervolgens set ik de achternaam vp en dan gaat ie weer genereren.


Ja, maar dat moet je evengoed doen, welke oplossingsmogelijkheid je ook kiest. Anders krijg je de situatie dat je een keer die $fullname hebt gegenereerd en vervolgens de voornaam of achternaam aanpast. Je getFullName() geeft dan nog steeds de oude naam terug, omdat $fullname immers al geset is.
 
Ozzie PHP

Ozzie PHP

15/04/2014 16:39:37
Quote Anchor link
Dan kan ik het dus beter "on the fly" genereren zoals Ger zegt en dus niet opslaan in een aparte class property.
 



Overzicht Reageren

 
 

Om de gebruiksvriendelijkheid van onze website en diensten te optimaliseren maken wij gebruik van cookies. Deze cookies gebruiken wij voor functionaliteiten, analytische gegevens en marketing doeleinden. U vindt meer informatie in onze privacy statement.