CODEWITHSUNDEEP
phpoop
LauraTheExplorer
LauraTheExplorer

Reputation: 852

When should I set my class properties in PHP - in constructor or as and when they are needed?

I am learning OOP PHP and I am struggling to know when to set my variables.

I've got a bunch of variables in my class which are used by some but not all functions. The code below shows how if a function needs a variable it

1) Checks if it is currently set

2) If it is not set, it runs whatever function sets that variable

Is this right, or should I set all variables using the __constructor function?

Thanks so much in advance - laura.

class Person {

private $user_id;
private $eye_color;

public function __construct ($user_id) {

    $this->user_id = $user_id;

}


public function eyeColor () {

    $this->eye_color = get_user_meta($this->user_id, '_eye_color', 'true');

}

public function describeEyes () {

    // If Eye Color Is not set, set it
    if (!isset($this->eye_color)) {
        $this->eyeColor();
    }

    $eye_decription = 'The user has beautiful eyes, they are' . $this->eye_color;

    return $eye_decription;

}

public function describeFace () {

    // If Eye Color Is not set, set it
    if (!isset($this->eye_color)) {
        $this->eyeColor();
    }

    $face_decription = 'The user has a nice face and beautiful eyes, they are' . $this->eye_color;

    return $face_decription;
}

}

Upvotes: 2

Views: 60

Answers (2)

e_i_pi
e_i_pi

Reputation: 4820

In your case, I would argue that your Object Composition of the Person class is off. If "eye color" belongs to the user, then that's where it should remain, and it should only be exposed by the Person class, not duplicated across both classes. See the below code changes:

class Person {

    private $user_id;

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

    public function eyeColor()
    {
        return get_user_meta($this->user_id, '_eye_color', 'true');
    }

    public function describeEyes()
    {
        return 'The user has beautiful eyes, they are' . $this->eyeColor();
    }

    public function describeFace()
    {
        return 'The user has a nice face and beautiful eyes, they are' . $this->eyeColor();
    }

}

A few things to note here:

  1. We are only ever setting user_id via the constructor. This implies that a Person object cannot exist without having a user_id, which achieves the composition you are after.
  2. We are exposing get_user_meta($this->user_id, '_eye_color', 'true') via the function eyeColor(), meaning that the eye color of a Person is actually just a reference to the user's eye color attribute.
  3. We will never have a mismatch between a persons eye color and the underlying user's eye color, as we cannot change a person's eye color independently of the user's eye color.

I would argue further that you should, instead of passing in a user_id, pass in a User object, but this would depend on how OO your code is and how far you want to take it.

Upvotes: 1

AngelGris
AngelGris

Reputation: 813

I don't think there's a unique way of doing this and it depend on each case.

If the function loading the data (in your case eyeColor()) is expensive in terms of resources or time, then making it only run when required is a good choice. If it's something simple then you can run it in the constructor.

Also keep in mind that if you are going to use the eye_color may times, each time you need it you'll be testing a conditional clause to see if it's already loaded. So if you use this value too many times it could be better to load it in the constructor and save those conditional testings.

If you are sure you'll use it at least once better put it in the constructor.

Upvotes: 2

Related Questions