Validating constructor parameters, making sure parameters have the correct type

Writing my first PHP Class and came across an issue, the following is my __construct method:

public function __construct($foo,$bar) {
       $this->foo = $foo;
       $this->bar = $bar;
}

Both $foo and $bar are required, without them, the methods will not work. It's OK when they aren't defined when the object is instantiated like so:

$var = new Class();

As this throws an exception (e.g. Class requires 2 开发者_如何学编程params, none set). But if they are set but not of the correct type like so:

$var = new Class('33','ddd');

My methods will fail as the variables have the wrong type.

Where should I be validating these? In the constructor or in each method?

My solution that I'm using right now works, but I'm not sure if it's the right way:

// $foo needs to be a string with letters only
// $bar needs to be an integer
public function __construct($foo,$bar) {
       $this->foo = $foo;
       $this->bar = $bar;
       if(!is_numeric($bar)){
           // Throw exception
       }
       elseif(other validation case)
       etc...
}

The concepts of OO programming are pretty new to me, so links to any reference material you have would be greatly appreciated.

I'd probably do something like this to prevent clutter inside the ctor and to allow the class to set them values internally:

class MyClass …

    protected $_foo;

    /**
     * @param  String   $foo    String with letters only
     * @param  Integer  $bar    Any Integer
     * @return void
     * @throws InvalidArgumentException when $foo is not letters only
     * @throws InvalidArgumentException when $bar is not an Integer
     */
    public function __construct($foo, $bar) 
    {
        $this->_setFoo($foo);
        $this->_setBar($bar)
    }

    /**
     * @param  String   $foo    String with letters only
     * @return void
     * @throws InvalidArgumentException when String is not letters only
     */
    protected function _setFoo($foo)
    {
        if (FALSE === $this->_consistsOfLettersOnly($foo)) {
            throw new InvalidArgumentException(
                '$foo should consists of letters only'
            );
        }
        $this->_foo = $foo;
    }  

    …

This has the added advantage that if you need to publicly expose the setters later, you just have to change the visibility keyword.

Making the validation into it's own method is not strictly necessary, but I think it makes the code more readable. If you find another property in that class needs the same validation, you can also reuse it more easily.

Validating the types in the constructor (as per your example) seems the logical choice to me, although it's a bit odd if the arguments are optional. You should probably set safe defaults if this is case via...

public function __construct($foo=null, $bar=null)

That said, you can use type hinting for arguments in PHP, but I believe this only works for arrays and classes. (Not a surprise, as there's no such thing as an integer or string type in PHP.)

As others have said, you should also ensure that you carry out validation in any setters you have or (better still) simply call the setters from within the constructor to ensure there's no duplication of code.

If $foo and $bar are required, validating them in __construct is the best place to do so (if not: use a setter and validate there). If you do it in every method, you'll just end up with duplicted code. Imagine if a datatype changes, you'd have to change every method ...

If you like to be really picky, you could call is_numeric before assigning $bar to $this->bar as the assignment is not needed if the check for a numeric value fails.

The best way is to use getters and setters - for example:

private $_foo;
private $_bar;

public function __construct($foo,$bar) {
  $this->setFoo($foo);
  $this->setBar($bar);
}

public function setFoo($foo) {
  // validate here

  $this->_foo = $foo;
}

public function getFoo() {
  return $this->_foo;
}

...

Much cleaner this way...

Your issues aren't stemming from the fact that you are writing object orientated code — it's because PHP is a loosely typed language. This means that variables can contain any kind of data, from int to Object. You can enforce some kind of type casting like so:

$foo = (int)$foo;
$bar = (str)$bar;

Elaborating a bit on @xil3's answer I'd say all Dan was looking for / needs is:

private $_foo;
private $_bar;

public function __construct($foo,$bar) {
  if(is_int($foo)) {
    $this->setFoo($foo);
  }
  if(is_string($bar)) {
    $this->setBar($bar);
  }
}

public function setFoo(int $foo) {
  $this->_foo = $foo;
}

public function setBar(string $bar) {
  $this->_foo = $bar;
}

...

Allowing his object to be instantiated parameterless without the fatal, with type constraints on the setters.

You should not do anything but setting values in constructor. If you need to validate constructor parameters to that in (possibly private) setters, or use arguments of particular type, so PHP will validate type for you, for instance:

__construct(SomeType $foo, AnotherType $bar);

For instance you can use SPL data types:

__construct(SplInt $integer, SplString $string);

Read more about SPL data types

declare('strict_types=1');

public function __construct(
    int $foo, 
    string $bar, 
    Request $request // type hint even classes
) {
       $this->foo = $foo;
       $this->bar = $bar;
}