Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

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

Tags:

oop

php

class

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?

like image 635
Nicola Peluchetti Avatar asked Jan 28 '26 08:01

Nicola Peluchetti


1 Answers

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.

like image 157
Dai Avatar answered Jan 29 '26 21:01

Dai