PHP OOP Constructor Design Pattern

Question

i have this code. It works, but is it possible to do this better ? With less code ?

public function __construct($type ,$brand, $model, $year, $mileage, $height, $width, $depth, $ps, $color, $price, $value, $seats)
{
    if (is_int($type)) {
        $this->_type = $type;
    } else {
        throw new Exception('Wrong Value in Type. Expected integer');
    }
}

...

I create an object like this

try {
    $firstCar = new Car(1, 'Audi', 'A3', 2011, 94512, 2, 2, 2, 105, 'Black', 1000, 1920, 5);
} catch ( Exception $error) {
    die($error->getMessage());
}

I repeat the "IF-ELSE" Statement for every variable in 2 Classes. Its so much code.

Some suggestions out here?

That feels like you're passing way too many arguments to the constructor: if you have to do this, rather than using setter methods for each property, perhaps looping over `func_get_args()` — Mark Baker, Aug 19 '14 at 13:11
It looks like a good question for http://codereview.stackexchange.com — Clément Malet, Aug 19 '14 at 13:13
Hey @MarkBaker. Maybe, but i cant find a better solution. Read a bit about Design patterns, but i dont know what to do with that code. — Webice, Aug 19 '14 at 13:13
I don't think it's the job of the class to validate input. That should have been done externally, before the information was every passed to the constructor. — Pagerange, Aug 19 '14 at 13:16
@Pagerange The problem is you dont have any Input. Its hard coded. Its a task, not more. I edit my code. Than you see it. — Webice, Aug 19 '14 at 13:18
@MarkBaker Is it better to have a setter method for every argument? And how should i do that setter methods? — Webice, Aug 19 '14 at 13:19
Following on @mark-baker comment, re: looping through func_get_args()... you could pass that array to a private method within the class that simply validates all elements. Or setters for each parameter that have their own validator. There's another discussion here about the same thing: http://stackoverflow.com/questions/15186202/should-i-validate-parameters-in-constructor — Pagerange, Aug 19 '14 at 13:24

score 1 · Accepted Answer · answered Aug 19 '14 at 13:13

1

Assuming, each field must be an int:

...

private function checkArgument($arg) {
    if (is_int($type)) {
        $this->_type = $type;
    } else {
        throw new Exception('Wrong Value in Type. Expected integer');
    }
}

...

public function __construct($type ,$brand, $model, $year, $mileage, ...) {
    checkArgument($type);
    checkArgument($brand);
    ...
}

You could also use func_get_args(), to get all argument in an array, and loop on them.

answered Aug 19 '14 at 13:13

Balázs Édes

13,452
6
54
89

That looks better, but not perfect. No shorter way ? – Webice Aug 19 '14 at 13:16
And i have integer, strings, boolean and floats. – Webice Aug 19 '14 at 13:22
2

Shorter: I don't think so. More elegant, yes: `Validator` interface, implementations for each type (`BooleanValidator`, `IntegerValidator`, ...). Map your parameters to a validator, loop over them, and validate each. An even more elegant version would be to use a statically typed language (like HACK - i don't know how mature it is), and let the interpreter do this whole stuff. – Balázs Édes Aug 19 '14 at 13:50

score 0 · Answer 2 · answered Aug 19 '14 at 14:34

I have the Problem solved differently.

$this->setType($type);

protected function setType($worth)
{
    if (is_int($worth)) {
        $this->_type = $worth;
    } else {
        throw new Exception('Wrong Value in Type. Expected valid integer');
    }
}

This for each argument i passed to the Class.

For those who are interested in that. Not the best Solution, but it shortens the child classes.

v2solutions.com · Answer 3 · 2014-08-20T07:45:33.687

For checking the values to be integer, you can use filter_var in callback way or you can pass array as arguments and use filter_var_array function in below way, below is my example code:

$firstCar = new Car(1, 'Audi', 'A3', 2011, 94512, 2, 2, 2, 105, 'Black', 1000, 1920, 5);

public function __construct($type ,$brand, $model, $year, $mileage, $height, $width, $depth, $ps, $color, $price, $value, $seats) {
$arrInput = func_get_args();

// this is enough to check only all arguments are integer or not by comparing the counts by input and filter array
$arrFilter = array_filter(filter_var_array($arrInput,FILTER_VALIDATE_INT));

 if (count($arrFilter) < count($arrInput)) { // means there should be atleast one non-integer value
    // to get the exact values which are exaclty not interger
    $arrResult = array_diff($arrInput, $arrFilter);
    throw new('your error'); // or return $arrResult; // this return Audi, A3 and Black
 }

 }

You can catch error using try/catch by throwing error from constructor or get the result values which are not integer by returning $arrResult from constructor, as per your need.

score 0 · Answer 4 · answered Apr 16 '15 at 10:23

It's not a lot of code, just messy.

Probably the problem is too many things in one place, group "types" of properties in values object, then each of these value object will handle their "validation"

example:

height, width, depth should be Size class
type, brand, model, year should by CarModel
price, value should go to Price
...

or something similar, you should know how they relate

would result in:

function __construct(CarType $model, Size $size, Price $price, $seats)
{
    // no need to validade CarType did that
    $this->model = $model;
    ....
}

To help in this object creation you could use a factory or a builder

PHP OOP Constructor Design Pattern

4 Answers4