Is it a better practice to inject all variables in the constructor or to use setters and throw exceptions if they are not set?

Question

Imagine you have this class

class Ai1ec_Less_Parser_Controller {
    /**
     * @var Ai1ec_Read_Variables_Startegy
     */
    private $read_variable_strategy;
    /**
     * @var Ai1ec_Save_Variables_Strategy
     */
    private $write_variable_strategy;
    /**
     * @var Ai1ec_Less_Variables_Collection
     */
    private $less_variables_collection;
    /**
     * @var Ai1ec_Less_Parser
     */
    private $ai1ec_less_parser;
    /**
     * We set the private variables in the constructor. I feel that there are too many parameters. 
     * Should i use setter instead and throw an exception if something is not set?
     * 
     * @param Ai1ec_Read_Variables_Startegy $read_variable_strategy
     * @param Ai1ec_Save_Variables_Strategy $write_variable_strategy
     * @param Ai1ec_Less_Variables_Collection $less_variables_collection
     * @param Ai1ec_Less_Parser $ai1ec_less_parser
     */
    public function __construct( Ai1ec_Read_Variables_Startegy $read_variable_strategy,
                                 Ai1ec_Save_Variables_Strategy $write_variable_strategy,
                                 Ai1ec_Less_Variables_Collection $less_variables_collection,
                                 Ai1ec_Less_Parser $ai1ec_less_parser ) {

    }
}

I need those variables to be set and so i set them in the constructor ( but that look like too many parameters ). Another option would be to use setters to set them and then in a method throw an exception if one of the required variables is not set like this

public function do_something_with_parser_and_read_strategy() {
  if( $this->are_paser_and_read_strategy_set === false ) {
     throw new Exception( "You must set them!" );
  }
}  
private function are_paser_and_read_strategy_set () {
  return isset( $this->read_variable_strategy ) && isset( $this->ai1ec_less_parser );
} 

Do you think that one of the two methods is better?And why?

Answer 1

There is no "better" way. But here are few things you have to consider:

• constructors are not inherited
• if class requires too many objects, it is responsible for too much

This might have impact on your choice of what sort of interface your class implements.

The general rule of thumb would be this:

If parameters are mandatory for class to function, they should be injected through constructor.

The exception would be, if you initialize the instance by using a factory. It is quite common for factory to build instance form diverse classes, where some of them implement same interface and/or extend same parent class. Then it is easier to inject shared objects through setters.

Answer 2

The standard solution to "too many constructor arguments" is the builder pattern. Your controller class itself will still have a long constructor, but clients can use setters on the builder, which in turn will later call the long constructor.

If you only construct your controller object in one or two places, it wouldn't even be worth all the trouble to create a builder; in that case, just stick with your current code.

Answer 3

Class naming Controller somehow reflects MVC, or in general - any mechanism responsible for processing sequence.

Data object classes tend to have many fields in any case - it is their responsibility. Regular object relying on many other objects could be possibly missing a point.

There are four objects, as I see: read, save, parse and provide collection interface to something. Why shall one have different interfaces for reading and writing? Could this not be combined into one? Parser shall be a library on itself, thus there may be no reason to combine it anywhere, although it could possible use readers/writers for itself, and, in return, provide collection. Thus could it be possible that parser would take an argument of reader and return a collection object?

That is more about specific case. In general - having many arguments to the method (or initializing many fields within an object by other objects of different domains) indicates some sort of design flaw.

Kind of on-topic might be this article on Constructor Initialization - it advises to use in-constructor initialization. Just be sure to follow up to the point:

What if there's a lot of collaborators to provide in the constructor? A large list of construction parameters, like any large parameter list, is a CodeSmell.

And as Ray has written - there is a possibility to initialize using setters, and there is article on that too. To the extent of my view - I think that Martin Fowler really summarizes these cases pretty well.

Answer 4

Any function that has over 2 (sometimes 3) arguments, I always pass an array, so it would look like:

public function __construct(array $options = array()) {

    // Figure out which ones you truly need
    if ((!isset($options['arg1'])) || (mb_strlen($options['arg1']) < 1)) {
        throw new Exception(sprintf('Invalid $options[arg1]: %s', serialize($options)));
    }

    // Optional would look like
    $this->member2 = (isset($options['arg1'])) && ((int) $options['arg2'] > 0)) ? $options['arg2'] : null;

    // Localize required params (already validated above)
    $this->member1 = $options['arg1'];
}

Passing an array of options allows for future growth without having to change the function signature. However it does have it's drawback in that the function must localize all elements of the array to ensure access doesn't throw warnings / errors (if an element is missing from the array).

The factory solution is in this case is not a good choice, because you are still left with the problem of passing the values to the factory so it can initialize the object with the correct values.

Answer 5

Is your class immutable? If so, then having 100% member population via the constructor is often the best way to do it, but I'll agree it can start to look ugly if you have more than a 5 or 6 parameters.

If your class is mutable then there's no benefit from having a constructor with required parameters. Expose the members via accessor/mutator methods (aka properties).

The factory pattern (as suggested by @Ray) can help, but only if you have a variety of similar classes - for a one-off then you can simply use static methods to instantiate the object, but you'll still have the "too many parameters" problem.

The final alternative is to accept an object with fields (one field for each parameter), but use this technique carefully - if some values are optional then just use method overloading (which unfortunately PHP doesn't support).

I'd just stick with what you're doing and only change it for something else if it presents a problem.

Answer 6

Creating your objects using factories that call setters instead of using a constuctor of a set number of parameters is much more flexible. Check out the builder and factory patterns.

Throwing exceptions for accessing not fully built objects is good!