Geeks With Blogs
Chris Falter .NET Design and Best Practices

All of us have probably written code like this:

Foo f = new Foo();

And what could be simpler?  As long as the logic in the constructor is simple (or better yet, the constructor is empty), it would seem that the simplest code is the best, so just use the constructor.  Certainly the MSDN documentation is rife with code that uses public constructors.  You can probably find plenty of public constructors used right here on my blog.  Why invest the effort in writing (and using) a factory class that will probably never do anything useful, other than call a public constructor?

In his excellent podcast entitled "Emergent Design: The Evolutionary Nature of Software Development," Scott Bain of Net Objectives nevertheless makes a strong case against the routine use of public constructors.  The problem, notes Scott, is that the use of a public constructor ties the calling code to the implementation of Foo as a concrete class.  But suppose that you later discover that there need to be many subtypes of Foo, and Foo should therefore be an abstract class instead of a concrete class--what then?  You've got a big problem, that's what; a lot of client code that has been making use of Foo's public constructor suddenly becomes invalid.

Exposing a public constructor, observes Bain, allows code to violate the open/closed principle.  This principle states that software should be open for extension, but closed for modification.  Restating this in more mundane terms, developers should design their code in such a way that extending it (for example, making a class more useful by defining it as abstract and subclassing it as appropriate) does not provoke side effects elsewhere--and that's the "closed for modification" part in a nutshell.

Bain points out that there is a simple, low-cost practice that avoids the public constructor's violation of the open/closed principle.  Just give a class constructor protected scope, and define a static Create method that uses the constructor--that's it.  Here's class Foo's new skeletal definition in C#:

public class Foo
{
  protected Foo() {}

  public static Foo Create() { return new Foo(); }
}

How hard was that?  At the cost of a single line of code, you get benefits well beyond the already mentioned ability to transform the class into an abstract base class:

  • If object construction becomes more complex, you can use a factory class inside the Create method.
  • You can implement the Dependency Injection pattern, which facilitates unit testing with mock objects.
  • You can use Microsoft's Policy Injection Application Block to implement cross-cutting concerns such as logging, authorization, and validation.

Unfortunately, you may occasionally encounter a corner case where you must define a public constructor.  The .NET Framework's XmlSerializer cannot be used on a class with no public default constructor, since XmlSerializer must instantiate the class in order to discover the public properties that must be serialized.  If you plan to serialize a class using XmlSerializer, then, you must make the constructor public--but you should mark it with the ObsoleteAttribute to warn fellow developers not to use it:

 public class Foo
{

  [Obsolete("Do not use--provided only for use by XmlSerializer.  Client code should call Foo.Create().")]
  public Foo() {}

  public static Foo Create() { return new Foo(); }
}

If due to habit a fellow programmer calls the public constructor (against your intentions), the C# compiler will emit a helpful warning message.  Of course, there are programmers who ignore warnings, but that is the subject of a different post.

Posted on Friday, February 15, 2008 2:59 PM Coding Practices and Design Patterns | Back to top


Comments on this post: "New" Statement Considered Harmful

# re: "New" Statement Considered Harmful
Requesting Gravatar...
Chris:

Thanks for the addition of the "Obsolete" attribute. I'll pass that along to my .Net students.

I also offer this notion in addition:

When working in managed code, this argument is easier to make than when discussing issues like this with C++ devs. They are concerned that the issue of memory cleanup will get muddy when creation is encapsulated like this.

The answer? The general idea of encapsulation still applies, but is extended to the routine use of "ReturnInstance(Foo f)" (or s similar name) to allow clients to return the instance when done with it, and then we encapsulate what does/should happen at that point.

This also allows a standard (not reused) class to become a Singleton (reused) later without breaking clients, and vice-versa.

I also generally prefer this "GetInstance(), ReturnInstance(Foo f)" pair of methods over the practice of creating objects on the stack, and letting the stack clean them up when the scope closes. This also creates concrete coupling (like "new" does), and also makes the point in the code where the object is no longer available much less explicit (you have to track the scope). A method call that says "I am done with it here" is much easier to pick out in the code.

-Scott-
Left by Scott Bain on Feb 15, 2008 3:26 PM

# re: "New" Statement Considered Harmful
Requesting Gravatar...
Frankly, I've heard this argument a lot in the last years, and it never has made sense to me.

That is, I agree that "new Foo()" couples the client to the concrete implementation of Foo, of course. And from time to time, I find that to make the code more flexible, I need to abstract away from that call.

In those cases, though, I almost never find that it's the class Foo who should have the responsibility to decide which implementation the client should use. Almost always it should be the responsibility of a third class. A classical example is the use of dependency injection.
Left by Ilja Preuß on Feb 16, 2008 6:50 AM

# re: "New" Statement Considered Harmful
Requesting Gravatar...
Scott -

Thanks for the C++ observations. They make me appreciate the .NET Framework's garbage collection all the more! I know not everyone can work in Java or .NET, though, so your comment is quite useful.

You did mention another extension scenario that I missed: what if you want to change class Foo so that it is a singleton? Avoiding the use of "new" facilitates that modification, as well.

Ilja -

The extension scenarios I had in mind mostly involved using a third class (a factory) to construct an instance of Foo. Just modify the Create method so that it calls the factory's Create method:

public static Foo Create()
{
return FooFactory.Create();
}

If you maintain the practice of hiding the constructor and supplying a Create method, abstracting construction into a factory is much, much simpler--you only have to make the change in one place (inside the Create method) rather than in dozens of places.

To make this work in the .NET Framework, though, you'd need to define the constructor's scope as protected internal, and compile FooFactory in the same assembly.
Left by Chris Falter on Feb 16, 2008 7:54 AM

# re: "New" Statement Considered Harmful
Requesting Gravatar...
There is, actually, one exception to the concrete coupling created by new. Anyone care to venture a guess where it is? :)

-S-
Left by Scott Bain on Feb 16, 2008 10:41 AM

# re: "New" Statement Considered Harmful
Requesting Gravatar...
You've got to be thinking of the situation where Foo is a struct rather than a class. There is no such thing as an abstract struct (or a singleton struct), so in theory you're not worried about the effect of later refactorings.

However, you are still tying yourself to the implementation of Foo as a struct rather than a class.
Left by Chris Falter on Feb 19, 2008 12:07 AM

# re: "New" Statement Considered Harmful
Requesting Gravatar...
How does this pattern work with inheritance?
Left by Mike Thomas on Feb 19, 2008 5:27 AM

# re: "New" Statement Considered Harmful
Requesting Gravatar...
Mike -

I don't think I'd call this a pattern; rather, I'd call it a practice.

As far as inheritance goes, the practice allows us to modify class Foo into a superclass (or abstract class) at little expense. So class Foo could have subclasses, and the Foo.Create method would return the appropriate subclass based on environment/configuration/etc.

abstract public class Foo
{
private const string KIND = "Kind";
private const string VICIOUS = "Vicious";

protected internal Foo() { }
public static Foo Create()
{
switch (Environment.GetMood())
{
case KIND: return new KindFoo();
case VICIOUS: return new ViciousFoo();
default: throw new ApplicationException("Environment.GetMood returned unknown setting");
}
}

abstract public string Description { get; }
}

public class ViciousFoo: Foo
{
protected internal ViciousFoo(): base() { }

override public string Description { get { return "I step on ants for sport. Mwa-ha-ha-ha!" } }

}

public class KindFoo: Foo
{
protected internal KindFoo(): base() { }

override public string Description { get { return "I help old ladies cross the street." } }

}
Left by Chris Falter on Feb 19, 2008 10:42 AM

# re: "New" Statement Considered Harmful
Requesting Gravatar...
Actually, I was thinking of constrained generic types:

class Foo<T> where T:new() {
T myService;
public Foo() {
myService = new T();
}
}

new T() is not a concrete reference, but a way to build anything that qualifies as T. If you further contrain to an interface, you have parametric polymorphism in the truest sense of the word.

Constrained generics can be used to implement most design patterns, and can have large performance benefits.

-S-
Left by Scott Bain on Feb 19, 2008 11:45 AM

# re: "New" Statement Considered Harmful
Requesting Gravatar...
Scott,

Please correct me if I've misunderstood, but what you just said doesn't seem to make any sense.

You would use the class you provided as an example as follows:

var foo = new Foo<SomeType>();

If you do this you are still bound to a concrete implementation of SomeType, are you not? You said "new T() is not a concrete reference", but by its very nature the "new()" constraint constrains the type parameter to a concrete type.
Left by Nick on Feb 19, 2008 1:37 PM

# re: "New" Statement Considered Harmful
Requesting Gravatar...
Let's say I build a class called Bar. I also build another call Boo. They both have default constructors.

I can do this:

myFoo = new Foo<Bar>();

...and myFoo will talk to an instance of Bar.

or...

myFoo = new Foo<Boo>();

...and myFoo will talk to an instance of Boo.

Therefore, Foo, even though it contains "new", is not coupled to Bar, nor Boo, or any other concrete type. This becomes clear when, later, I do:

myFoo = new Foo<Bee>();

...and I make no changes to Foo whatsoever, so long as Bee fits the constraint (in this case, a default constructor).

This is the ONLY case I can think of where the use of new does not couple to a specific concrete type.

I mentioned it mostly as an interesting fact. It's a Strategy Pattern without a virtual method call.

Of course, to make it really work, I'd need to do this in Foo:

public class Foo<T> where T:new(), IStrgy

Which would ensure that Bar, Boo, and the new Bee also implement some known interface. This way, Foo could interact with them in meaningful ways, again without coupling to them concretely.

-S-
Left by Scott Bain on Feb 19, 2008 1:52 PM

# re: "New" Statement Considered Harmful
Requesting Gravatar...
By not exposing the constructor you lose the ability to use generics in object creation.

For example, you can no longer write x = new T(). This can be really important for things like databound lists.
Left by Jonathan Allen on Feb 19, 2008 7:03 PM

# re: "New" Statement Considered Harmful
Requesting Gravatar...
Interesting, i used this method and really liked it it is most deffinately effecient



Thanks for writing about it
Left by software development uk on Oct 01, 2009 11:01 AM

# re: "New" Statement Considered Harmful
Requesting Gravatar...
Know this post is old, but it comes out near the top when searching for "New Considered Harmful"
So thought I'd add that this doesn't work out to be so useful in practice: you're still coupling, and have just mixed in an extra factory responsiblity into the class.

New is however still harmful - it mixes in the responsiblity of knowing how to create an object and manage its lifetime into any code that uses it. Experience tells us this responsiblity is one of the most frequent ones to change.

The answer is even more simple - just don't do it.
Where you have
void DoSomethingWithX()
{
var x=new X();
x.DoIt();
}

You make
void DoSomethingWithX(X x)
{
x.DoIt();
}

Furtermore using dependency injection helps with creating and managing the lifetime of X.
Where the dependency on X is part of the objects state, you have it as a field and DI populates it (via constructor injection I would recommend).

public class Y
{
X x;
public Y(X x)
{
this.x=x;
}
void DoSomethingWithX()
{
x.DoIt();
}
}

In the rarer situation where the dependency must be created with dependent state, or its lifetime managed by the caller, think about responsiblities again - you're now dependent on something that can create an instance of X passing it some state - i.e. you're dependent on an XFactory. It's still a simple dependency, but a more clear responsiblity.

public class Y
{
XFactory xf;
public Y(XFactory xf)
{
this.xf=xf;
}
void DoSomethingWithX()
{
var x=xf.Create(some,state,here);
x.DoIt();
}
}
Left by andyclap on Oct 27, 2011 6:25 AM

Your comment:
 (will show your gravatar)


Copyright © Chris Falter | Powered by: GeeksWithBlogs.net