Object Oriented PHP Best Practices

Question

Say I have a class which represents a person, a variable within that class would be $name.

Previously, In my scripts I would create an instance of the object then set the name by just using:

$object->name = "x";

However, I was told this was not best practice? That I should have a function set_name() or something similar like this:

function set_name($name)
{
    $this->name=$name;
}

is this correct?

If in this example I want to insert a new "person" record into the db, how do I pass all the information about the person ie $name, $age, $address, $phone etc to the class in order to insert it, should I do:

function set($data)
{
    $this->name= $data['name'];
    $this->age = $data['age'];
    etc
    etc

}

Then send it an array? Would this be best practice? or could someone please recommend best practice?

Answer 1

Using explicit getters and setters for properties on the object (like the example you gave for set_name) instead of directly accessing them gives you (among others) the following advantages:

• You can change the 'internal' implementation without having to modify any external calls. This way 'outside' code does not need change so often (because you provide a consistent means of access).
• You provide very explicitly which properties are meant to be used / called from outside the class. This will prove very useful if other people start using your class.

The above reasons is why this could be considered best practice although it's not really necessary to do so (and could be considered overkill for some uses ; for example when your object is doing very little 'processing' but merely acts as a placeholder for 'data').

Answer 2

As a counterpoint to ChristopheD's answer, if your instance variable is strictly for private use, I wouldn't bother with writing a getter & setter, and just declare the instance variable private.

If other objects need to access the object, you can always add a getter. (This exposes another problem, in that other classes might be able to change the object returned by the getter. But your getter could always return a copy of the instance variable.)

In addition using a getter/setter also shields other parts of the same class from knowing about its own implementation, which I've found very useful on occasion!

Answer 3

I perfectly agree with CristopheD (voted up). I'd just add a good practice when creating a new person.

Usually, a use a constructor which accept the mandatory fields and set the default values for the optional fields. Something like:

class Person
{
  private $name;
  private $surname;
  private $sex;

  // Male is the default sex, in this case
  function Person($name, $surname, $sex='m'){
    $this->name = $name;
    $this->surname = $surname;
    $this->sex = $sex;
  }

  // Getter for name
  function getName()
  {
    return $this->name;
  }

  // Might be needed after a trip to Casablanca
  function setSex($sex)
  {
     $this->sex = $sex;
  }
}

Obviously, you could use the setter method in the constructor (note the duplicate code for the sex setter).

Answer 4

From a more general point of view both direct access ($person->name) and accessor methods ($person->getName) are considered harmful. In OOP, objects should not share any knowledge about their internal structure, and only execute messages sent to them. Example:

// BAD

function drawPerson($person) {
  echo $person->name; // or ->getName(), doesn't matter
}

$me = getPersonFromDB();
drawPerson($me);

// BETTER

class Person ....
   function draw() {
       echo $this->name;
    }

$me = getPersonFromDB();
$me->draw();

more reading: http://www.javaworld.com/javaworld/jw-09-2003/jw-0905-toolbox.html

Answer 5

You should have getter/setter functions. But let's face it, they are a pain to set up and eventually you'll just start accessing members directly. If you're using PHP5 you can use its magic methods to fix this problem:

   protected $_data=array(); 
   public function __call($method, $args) {
        switch (substr($method, 0, 3)) {
            case 'get' :
                $key = strtolower(substr($method,3));
                $data = $this->_data[$key];
                return $data;
                break;
            case 'set' :
                $key = strtolower(substr($method,3));
                $this->_data[$key] = isset($args[0]) ? $args[0] : null;
                return $this;
                break;
            default :
                die("Fatal error: Call to undefined function " . $method);
        }
    } 

This code will run every time you use a nonexistent function starting with set or get. So you can now set/get (and implicitly declare) variables like so:

$object->setName('Bob');
$object->setHairColor('green');

echo $object->getName(); //Outputs Bob
echo $object->getHairColor(); //Outputs Green

No need to declare variables or setter/getter functions. If in the future you need to add functionality to a set/get method you simply declare it, essentially overriding the magic method. Also since the setter functions return $this you can chain them like so:

 $object->setName('Bob')
        ->setHairColor('green')
        ->setAddress('someplace');

Which makes for code that is both easy to write and easy to read.

The only downside of this approach is that it makes your class structure more difficult to discern. Since you're essentially declaring members/methods on run time, you have to dump the object during execution to see what it contains, rather than reading the class. If your class needs to declare a clearly defined interface (because it's a library and/or you want phpdoc to generate the API documentation) I'd strongly advice declaring public facing set/get methods along with the above code.

Answer 6

To go full OOP, you should do something similar to:

class User {

private $_username;
private $_email;

public function getUsername() {
    return $this->_username;
}
public function setUsername($p) {
    $this->_username = $p;
}
...
public function __construct() {
    $this->setId(-1);
    $this->setUsername("guest");
    $this->setEmail("");
}
public function saveOrUpdate() {
    System::getInstance()->saveOrUpdate($this);
}
}

If you want to save a user, you just create one, assign its values using Setters and do $user->saveOrUpdate(), and have another class to handle all the saving logic.