views:

529

answers:

12

I started off by drafting a question: "What is the best way to perform unit testing on a constructor (e.g., __construct() in PHP5)", but when reading through the related questions, I saw several comments that seemed to suggest that setting member variables or performing any complicated operations in the constructor are no-nos.

The constructor for the class in question here takes a param, performs some operations on it (making sure it passes a sniff test, and transforming it if necessary), and then stashes it away in a member variable.

I thought the benefits of doing it this way were:

1) that client code would always be certain to have a value for this member variable whenever an object of this class is instantiated, and

2) it saves a step in client code (one of which could conceivably be missed), e.g.,

$Thing = new Thing;
$Thing->initialize($var);

when we could just do this

$Thing = new Thing($var);

and be done with it.

Is this a no-no? If so why?

A: 

You should not put things in a constructor that is only supposed to run once when the class is created.

To explain.

If i had a database class. Where the constructor is the connection to the database So

$db = new dbclass;

And now i am connected to the database.

Then we have a class that uses some methods within the database class.

class users extends dbclass
{
    // some methods
}

$users = new users 
// by doing this, we have called the dbclass's constructor again
Ólafur Waage
In that case I'd rather pass in an instance of dbclass, or use it as a singleton, rather than extend the class itself.
Ross
Yes there are ways around that, but this is just an example.
Ólafur Waage
If you didn't want that functionality you can override the constructor in the users object. Also some languages, such as C#, don't inherit the constructor at all.
Gary Willoughby
+6  A: 

Constructors are for initializing the object, so

$Thing = new Thing($var);

is perfectly acceptable.

Gary Willoughby
+16  A: 

My rule of thumb is that an object should be ready for use after the constructor has finished. But there are often a number of options that can be tweaked afterwards.

My list of do's and donts:

  • Constructors should set up basic options for the object.
  • They should maybe create instances of helper objects.
  • They should not aqquire resources(files, sockets, ...), unless the object clearly is a wrapper around some resource.

Of course, no rules without exceptions. The important thing is that you think about your design and your choises. Make object usage natural - and that includes error reporting.

gnud
A: 

I don't see why not, I find supplying lots of arguments using separate methods to be a bit of a drag, especially if there's no chaining involved.

Ross
+2  A: 

To improve the testability of a class it is generally a good thing to keep it's constructor as simple as possible and to have it ask only for things it absolutely needs. There's an excellent presentation available on YouTube as part of Google's "Clean Code Talks" series explaining this in detail.

cg
+1 I agree. We have tons of code where constructors actually open files directly to read some configuration info. In some ways, it makes these objects convenient to use but writing unit tests would be impossible. Good thing we don't have unit tests ;-)
Outlaw Programmer
+8  A: 

This comes up quite a lot in C++ discussions, and the general conclusion I've come to there has been this:

If an object does not acquire any external resources, members must be initialized in the constructor. This involves doing all work in the constructor.

  • (x, y) coordinate (or really any other structure that's just a glorified tuple)
  • US state abbreviation lookup table

If an object acquires resources that it can control, they may be allocated in the constructor:

  • open file descriptor
  • allocated memory
  • handle/pointer into an external library

If the object acquires resources that it can't entirely control, they must be allocated outside of the constructor:

  • TCP connection
  • DB connection
  • weak reference

There are always exceptions, but this covers most cases.

Tom
A: 

Depends on what type of system you're trying to architect, but in general I believe constructors are best used for only initializing the "state" of the object, but not perform any state transitions themselves. Best to just have it set the defaults.

I then write a "handle" method into my objects for handling things like user input, database calls, exceptions, collation, whatever. The idea is that this will handle whatever state the object finds itself in based on external forces (user input, time, etc.) Basically, all the things that may change the state of the object and require additional action are discovered and represented in the object.

Finally, I put a render method into the class to show the user something meaningful. This only represents the state of the object to the user (whatever that may be.)

__construct($arguments)
handle()
render(Exception $ex = null)

thesmart
A: 

The __construct magic method is fine to use. The reason you see initialize in a lot of frameworks and applications is because that object is being programmed to an interface or it is trying to enact a singleton/getInstance pattern.

These objects are generally pulled into context or a controller and then have the common interface functionality called on them by other higher level objects.

Syntax
+1  A: 

You should definitely avoid making the client have to call

$thing->initialize($var)

That sort of stuff absolutely belongs in the constructor. It's just unfriendly to the client programmer to make them call this. There is a (slightly controversial) school of thought that says you should write classes so that objects are never in an invalid state -- and 'uninitialized' is an invalid state.

However for testability and performance reasons, sometimes it's good to defer certain initializations until later in the object's life. In cases like these, lazy evaluation is the solution.

Apologies for putting Java syntax in a Python answer but:

// Constructor
public MyObject(MyType initVar) {
      this.initVar = initVar;
}

private void lazyInitialize() {
    if(initialized) {
        return
    }
    // initialization code goes here, uses initVar
}

public SomeType doSomething(SomeOtherType x) {
    lazyInitialize();
    // doing something code goes here
}

You can segment your lazy initialization so that only the parts that need it get initialized. It's common, for example, to do this in getters, just for what affects the value that's being got.

slim
*python => php
Joe Philllips
+3  A: 

The job of a constructor is to establish an instance's invariants.

Anything that doesn't contribute to that is best kept out of the constructor.

Morendil
A: 

If $var is absolutely necessary for $Thing to work, then it is a DO

Chepech
A: 

This article on Dependency Injection is rather helpful in relation to the constructor question. In a nutshell, the author recommends the use of setters to decouple a class from external resources at instantiation - and to delegate object instantiation (and the suite of setters you need to establish external dependencies) to a factory object.

http://www.potstuck.com/2009/01/08/php-dependency-injection/

sunwukung