Beautiful Constructors Look Like This

Constructors are great. As I’ve said before, I’m deadly against setters in code. We should be aiming for immutability in our applications wherever possible, and setters are the nemesis of this. Which means that we should be aiming to set everything up in the constructor.

Furthermore, this is the only thing that the constructor should be doing. Do not place any complex logic into the constructor of your object. It should be a place purely for setting the fields up of your object. Anything else that needs to be done — connections to other services for example — should always be part of some sort of init method. Preferably, the init method should have no parameters and everything needed to get the object into the correct state should be passed in as part of the constructor.

public class DoesSomeComplexConnection {

private final String dbUrl;
private final String user;

public DoesSomeComplexConnection(String dbUrl, String user){
    this.dbUrl = dbUrl;
    this.user = user;
}

public void init(){
    connectToDb(dbUrl, user);
}

As a rule, aim to have a solitary constructor. This makes understanding the code much easier. Your objects should not have “optional” dependencies- Your object should have a set of functionality which is always executed and relies on a certain set of dependencies. If you, the caller, do not need that functionality, then pass in no-op functionality.

    public DoesSomeComplexConnection(String dbUrl, String user, Notifier notifyOnConnection){
    this.dbUrl = dbUrl;
    this.user = user;
    this.notifyOnConnection = notifyOnConnection;
}

public void init(){
    connectToDb(dbUrl, user);
    notifyOnConnection.connected();
}

 public static void main(String[] args) {
    new DoesSomeComplexConnection(dbUrl, user, new DoNothingOnNotification())

}

private static class DoNothingOnNotification implements Notifier{
    @Override
    public void connected() {
    }
}

private interface Notifier {
    void connected();
}

This can be made even clearer in Java 8 with an anonymous no-op function.

        new DoesSomeComplexConnection(dbUrl, user, () -> {});

One of the most important parts of your code, which I will go into in a seperate article, is that you should never pass null around. Friends don’t let friends use null. By establishing this simple rule on your codebase means you can remove an awful lot of null checks that simply don’t need to exist.

If your codebase is irreparable, then put your null checks in the constructor. Make sure that by the end of construction all your internal fields are set and your object is complete.

Sometimes the answer to this is viewed as multiple constructors, which provide default implementations. Multiple constructors really clutter up the code base and make understanding difficult as mentioned before. It’s absolutely fine however to make default implementations available for the end user, like so:

         public class DoesSomeComplexConnection {

             public static final Notifier NO_OP_NOTIFIER = () -> {};
                …
                }

        new DoesSomeComplexConnection(dbUrl, user, DoesSomeComplexConnection.NO_OP_NOTIFIER);

A Practical Example:

Let’s take a look at the code for Reader from swagger-core:

public class Reader { 
public Reader(Swagger swagger) { 
this(swagger, null); 
}

public Reader(Swagger swagger, ReaderConfig config) {
    this.swagger = swagger == null ? new Swagger() : swagger;
    this.config = new DefaultReaderConfig(config);
}
}


public class DefaultReaderConfig implements ReaderConfig {
     /**
 * Creates default configuration.
 */
  public DefaultReaderConfig() {
}

/**
 * Creates a copy of passed configuration.
 */
public DefaultReaderConfig(ReaderConfig src) {
    if (src == null) {
        return;
    }
    setScanAllResources(src.isScanAllResources());
    setIgnoredRoutes(src.getIgnoredRoutes());
}

This code is particularly heinous for a number of reasons: 

• Nulls are being thrownn around left right and center. This makes it difficult to comprehend what’s going on and results in extra code for null checks. 

• Multiple constructors, particularly when the first is just passing another null!

• Construction of another Object in the constructor- this isn’t that big a sin, and is one that you need to apply common sense to. However, as it’s only accepting the one parameter it’s effectively decorating. Why not pass the required object in? 

• In likelihood, it is passing the required object in. But they’re wrapping it again just to be safe.

First of all, we can delete the first constructor from reader. Anyone requiring it is brave enough to pass the null in themselves (although we’ll be getting rid of nulls in due course).

Second, if we’re comfortable accepting people shouldn’t pass null around, we can remove the Swagger null check. If this is a problem the application will explode quite quickly and we can fix any other dependent code. This takes us to:

public class Reader {

public Reader(Swagger swagger, ReaderConfig config) {
    this.swagger = swagger;
    this.config = new DefaultReaderConfig(config);
}
}

Ok, getting better. Now, there is literally no need for the wrap call, it’s purely there as a null check again. DefaultReaderConfig is the only implementation available anyway. But let’s assume that’s not the case; we should change this code so that the object is passed directly in, not constructed internally:

    public class Reader {

public Reader(Swagger swagger, DefaultReaderConfig config) {
    this.swagger = swagger;
    this.config = config;
}
}

Ok, so our code is looking much cleaner here. What about if someone doesn’t have a config to pass through (e.g., would have been null before)? Default implementation.

public class Reader {
    public static final DefaultReaderConfig DEFAULT_CONFIG = new DefaultReaderConfig()

public Reader(Swagger swagger, DefaultReaderConfig config) {
    this.swagger = swagger;
    this.config = config;
}
}

//Someone using this class
new Reader(swagger, Reader.DEFAULT_CONFIG)

As a result of this, we can get rid of the 2 constructors in ReaderConfig too, as this was their only use.

Much cleaner and easy to understand code all round.