2

I have a PHP class that stores a complex multidimensional array, and rather than write individual accessor methods as an interface to this array I decided to use PHP5's __get method.

As I started to write these magic accessors it donned on me that I had no idea what best practices are here, so I thought I'd ask. Is one large if/else structure inside of __get() common?

I've included a small snippet of the class for reference:

<?php
   class Project {

      private $data; 

      public function __construct($d) {
         $this->data = $d;
      }

      public function __get($val) {
         if ($val === 'title')
            return $this->data['info']['projectName'];
         else if ($val === 'id')
            return $this->data['info']['projectID'];      
         else if ($val == 'health') {
            switch($this->data['info']['health']) {
               case 'OT':
               case 'NS':
               case 'LR':
                  return 'Low Risk&mdash;Healthy';
                  break;
               case 'CR':
                  return 'Critical Risk&mdash;Action Needed';
                  break;
               default:
                  return 'Unknown';            
            }
         }
      }
   }
?>

Then I'm just accessing the data like:

$o = new Project($array);
echo $o->title; #etc
hakre
  • 193,403
  • 52
  • 435
  • 836
JustinB
  • 101
  • 1
  • 6
  • This looks like a nightmare to maintain afterwards, not to speak of extending the monster. – aefxx Mar 05 '10 at 22:05
  • Don't speek if-else statement in __get() methods. I will say you should avoid use else-if statement as many as possible in everywhere. – caoglish Aug 03 '17 at 00:48

2 Answers2

0

In the end it is mostly an issue of alternatives. Therefore, I usually use the switch construct. Depending on the particular situation, regex can also be very useful.

txwikinger
  • 3,006
  • 1
  • 25
  • 33
0

No, it's not common in this way.

Adivce 1: __get() method should always return what you save in __set() section. This is a consistency requirement in general program.

If you want to return something processed base on the properties, I will suggest write in another methods rather than __get().

Adivce 2: Also, keep "single responsibility" in mind. If one method handle more than one functionality, it will be hard to reuse and modify later.

Adivce 3: About if ... else if ... else, I will call it else-if statement because it has more than two condition invovled. if...else I will call if statatement.

Don't speek else-if statement in __get() methods. I will say you should avoid use else-if statement as many as possible in everywhere. else-if is bad practise in general. Normally, in your if statmemt, when condition more than two, you should start to think abstract your code into better logic pattern/structure.The reason is that the else-if statment is hard to extend, maintain and unit test. Also else-if statement can always be converted into better design pattern.

Finally, is overwrite the magic getter and setter good Idea? It is a debt, please read this discussion:Best practice: PHP Magic Methods __set and __get, and make your mind.

caoglish
  • 1,343
  • 3
  • 19
  • 29