3

I create a class with strict properties' types, their values can be passed to constructor:

declare(strict_types=1);

class Position
{
    protected $hours;
    protected $minutes;
    protected $seconds;
    protected $frames;


    public function __construct(int $hours = 0, int $minutes = 0, int $seconds = 0, int $frames = 0)
    {
        $this->setHours($hours);
        $this->setMinutes($minutes);
        $this->setSeconds($seconds);
        $this->setFrames($frames);
    }

    public function getHours(): int
    {
        return $this->hours;
    }


    public function setHours(int $hours): void
    {
        $this->hours = $hours;
    }

    // and so on...
}

  1. Is it good to set properties' values with setters inside the constructor?

  2. Is it good to get their values inside class' methods with getters?

  3. Where it would be better to set the default properties' values: explicitly initialize properties with them, set them as default arguments values (as it's done for now) or specify them in both places?

Thanks.

Audiophile
  • 970
  • 2
  • 8
  • 20
  • 1. and 2. - that's why you use setters and getters - to not duplicate code. 3. - in initialization of properties (for not duplicating code) – Justinas Dec 06 '17 at 16:10
  • 3 - But then I need to set property only if the value was passed - this is additional portion of code inside the constructor. For example if I set argument int $hours = null, I need check: if (null !== $hours) { $this->setHours($hours) } - Do you think this is ok? – Audiophile Dec 06 '17 at 16:15
  • The constructor and the setters are redundant. Also, the default values ($int $hours = 0 and int $hours) are not identical. You should decide whether you want to create a DTO or a value object. – odan Dec 06 '17 at 16:20
  • @Daniel O. why redundant? I need to check type of values. And maybe I'll put some logic inside getters and setters in future. – Audiophile Dec 06 '17 at 16:23
  • I'd say avoid using non-final class methods in the constructor. You might forget you're doing it and override the methods somewhere else and get obscure errors. – apokryfos Dec 06 '17 at 16:24
  • 1
    I personnaly define 'default value' on constructor but only define type on setter. `public function __construct($hours = 0)` `public function setHours(int $hours): void` if you call constructor with a invalid type, setter will throw exception – Benjamin Poignant Dec 06 '17 at 16:26
  • Other than that you're writing the code, if the end result will be the same it doesn't matter, however if you want some intermediate logic to be performed before setting/getting then the answer is kind of obvious – apokryfos Dec 06 '17 at 16:28

1 Answers1

3

Is it good to set properties' values with setters inside the constructor?

In other words: is it ok to delegate to a setter when initializing a property in the constructor? The short answer is: It doesn't really matter. If there is no validation (other than the type) made, then there's really no point in it, but there's also no downside. If the is more validation to be made, then one could make the point that it keeps the constructor clean while reducing the amount of duplicate code. If there is additional checks to be made, then delegating to the setter for the common validation and implementing the additional one in the constructor will help keep the two workflows separate.

In the end, I don't think there's really a concern here. It's neither good nor bad practice to do it. Do what feels best for your specific use case.

Is it good to get their values inside class' methods with getters?

If you mean "should I access object properties through getters", then: Yes. A thousand times yes. Object properies should always be private and accessed through getters. Public properties violate one of the core concepts of Object-Oriented: Encapsulation. Whether or not each member has a getter and has logic or not is completely up to you and will depend on the use case.

if you mean "should I use the getter to access a propery from within the object" then it really doesn't matter and depends on the scenario. There may be times where using the public getter makes no practical sense and doesn't help the readability/maintainability of the code. There may be cases where it totally makes sense to call the public getter. For example when dealing with a function in the style of isFoo() which returns true or false based on a couple of conditions, it makes sense to simply call isFoo() instead of repeating the conditions again.

Where it would be better to set the default properties' values: explicitly initialize properties with them, set them as default arguments values (as it's done for now) or specify them in both places?

This one is a little more complex. In my mind there are three possible scenarios when choosing how to initialize an object property.

  1. The value MUST be passed to the constructor.

This is a case where a certain property must be specified when instantiating the object. If a value is not specified, the code should fail. For example, when creating an object of type User, then it is probably safe to assume it requires a firstName, lastName and email. If one or all of those variables are not passed to the constructor, the code should simply fail: the contract set by the constructor is not fulfilled.

class User {
    private $firstName;
    private $lastName;
    private $email;

    public function __construct(string $firstName, string $lastName, string $email) {
        $this->firstName = $firstName;
        $this->lastName = $lastName;
        $this->email = $email;
    }
}

$user = new User();

Exception: Argument 1 passed to User::__construct() must be of the type string, none given

  1. The value MAY be passed to the constructor.

This is a case where you want to allow a value to be set in the constructor, but is not required to instatiate the object. Taking the User example from before, we could also have an middleName property that a user can have, and we want to be able to set it the constructor.

class User {
    private $firstName;
    private $lastName;
    private $middleName;
    private $email;

    public function __construct(string $firstName, string $lastName, string $email, $middleName = null) {
        $this->firstName = $firstName;
        $this->lastName = $lastName;
        $this->email = $email;
        $this->middleName = $middleName;
    }
}

$user = new User('John', 'Doe', 'johndoe@email.com'); // does not throw an Exception

in my opinion there are very little reasons to actually implement a constructor in this way. It adds unnecessary complexity to the constructor while not actually solving any real problem. Sure it may be a quick and dirty way to add a parameter to the constructor without breaking any existing code, however if you are modifying the rules for creating an instance of an object, then the existing code should probably be updated to reflect this change.

  1. The value MUST be initialized to a default value.

This is a case where you want a certain property to be initialized to a specific default value as soon as the object is instantiated. This way there is no need to constantly check if the property has been initialized or not, or check for a null value when dealing with this property. If a specific use case requires that value to be overwritten right away, simply call the setter after instantiating the object.

class User {
    const USER_DEFAULT = 'default';

    private $firstName;
    private $email;
    private $propertyToInitialize = self::USER_DEFAULT;

    public function __construct(string $firstName, string $email) {
        $this->firstName = $firstName;
        $this->email = $email;
    }

    public function getInitializedProperty() {
        return $this->propertyToInitialize;
    }
}

$user = new User('John', 'johndoe@email.com');
echo $user->getInitializedProperty(); // outputs: 'default'

If you need a property's value to be part of a specific subset of values, I recommend checking out Enums

William Perron
  • 1,288
  • 13
  • 18