2

I am currently trying to refactor some code and one of the problems I came across was the constructors had far too many parameters (15 in fact) and was being initialised by another object which had the same amount of properties.

I am meant to be reducing the amount of classes we have so creating a set of classes which represented the different parts of the parameters seemed silly so I began looking and the best answer I came across was from this question on stackoverflow.

public class DoSomeActionParameters
{
    readonly string _a;
    readonly int _b;

    public string A { get { return _a; } }
    public int B{ get { return _b; } }

    DoSomeActionParameters(Initializer data)
    {
        _a = data.A;
        _b = data.B;
    }

    public class Initializer
    {
        public Initializer()
        {
            A = "(unknown)";
            B = 88;
        }

        public string A { get; set; }
        public int B { get; set; }
    }

    public static DoSomeActionParameters Create(Action<Initializer> assign)
    {
        var i = new Initializer();
        assign(i)

        return new DoSomeActionParameters(i);
    }
}

which can be called like so using a lambda

DoSomeAction(
    DoSomeActionParameters.Create(
        i => {
            i.A = "Hello";
        })
    );

I really liked this method even though it doesn't solve directly the problem of actually initialising the class, it does make it mean that I can have one constructor instead of 3. Also it doesn't require my class to know about objects it doesn't care about.

My problem is however, I do not know what this pattern is called so I can't do any further research on it to find out if it is still valid, is superseded by another pattern or whether it has sever performance issues. From my research the closest pattern I could find is the builder pattern but it seems to be more of an extension on that.

As a bonus point, it would be great if you could give me a hand with how to make certain parameters mandatory as we have about 10 parameters from the database and 5 more which are just flags which we don't really need to know about.

Keithin8a
  • 161

3 Answers3

13

I'd call this the "Elephant in the Room" (anti)pattern. You are focusing on the minutiae whilst ignoring the bigger problem. If you have a class that requires 15 constructor parameters, then this a warning sign that the class is doing too much and thus needs too much configuration.

The "pattern" you need here therefore is the Single Responsibility Principle. Examine the class, work out how many (likely quite distinct) things it's doing and start breaking those out into smaller, more focused, classes. Each will only then need a few constructor parameters.

To begin with, this will create more classes; but they'll be good classes. As you apply this to the mutated mess of similar classes, you'll be able to replace large parts of those with your new classes and what's left will again be better quality, more focused classes.

David Arno
  • 39,599
  • 9
  • 94
  • 129
3

Have a look at the Builder Pattern. It solves exactly the problem you are having - too many parameters in a constructor. You wind up with an empty constructor (typically in Java this will be a private method, if you use an inner class as the Builder) and setter methods (which may also remain private), then a Builder class with chained methods which call the setters, and a build() method which constructs the object and populates its fields.

1

I am creating my own answer (after a discussion on meta) because I believe that the correct answer is in fact a combination of everything so far.

Firstly as DavidArno put, the pattern that I have asked about is more of an anti-pattern because it is just hiding the problem somewhere else. However I believe the suggestion of using the builder pattern from the answer by Carl Manaster does solve the issue that I have because I cannot break down the list of parameters without over-engineering the Single Responsibility Principle.

The problem I had with the builder pattern was that there was no way to make it easy for a developer using it to know about the mandatory fields which will mean there will be lots of trial and error which will lead to copy and pasting the initialisation everywhere which will lead to more mistakes. Fuhrmanator however linked to the following blog which demonstrates how to have mandatory fields with the builder pattern (after looking elsewhere I believe this is called a Step Builder).

Essentially the principle of the Step Builder is to guide a developer through the initialisation of all the mandatory fields. Your builder implements various interfaces which act as steps to create the object. They each have a method which have a return type of the next step. This aids intellisense more than the standard builder patter as well because it cuts down the amount of methods you can call. See the following example from the blog.

public class Address {
private String protocol;
private String url;
private int port;
private String path;
private String description;

// only builder should be able to create an instance
private Address(Builder builder) {
    this.protocol = builder.protocol;
    this.url = builder.url;
    this.port = builder.port;
    this.path = builder.path;
    this.description = builder.description;
}

public static Url builder() {
    return new Builder();
}

public static class Builder implements Url, Port, Build{
    private String protocol;
    private String url;
    private int port;
    private String path;
    private String description;

    /** Mandatory, must be followed by {@link Port#port(int)}  */
    public Port url(String url) {
        this.url = url;
        return this;
    }

    /** Mandatory, must be followed by methods in {@link Build}  */
    public Build port(int port) {
        this.port = port;
        return this;
    }

    /** Non-mandatory, must be followed by methods in {@link Build}  */
    public Build protocol(String protocol) {
        this.protocol = protocol;
        return this;
    }

    /** Non-mandatory, must be followed by methods in {@link Build}  */
    public Build path(String path) {
        this.path = path;
        return this;
    }

    /** Non-mandatory, must be followed by methods in {@link Build}  */
    public Build description(String description) {
        this.description = description;
        return this;
    }

    /** Creates an instance of {@link Address} */
    public Address build() {
        return new Address(this);
    }
}

interface Url {
    public Port url(String url);
}

interface Port {
    public Build port(int port);
}

interface Build {
    public Build protocol(String protocol);
    public Build path(String path);
    public Build description(String description);
    public Address build();
}

You have the collection of Mandatory parameters which each return the next statement but also what I think is great about this is that you can have optional parameters as well by making the methods return type the build interface.

This solves my problem because I had mandatory parameters mixed in with optional ones which made expanding it difficult as there was lots of copy and pasting and potential problems. With the step builder I can control how a developer creates my object so that I have facts to use in the future, and I allow them the flexibility of creating the object in as many ways as they possibly need to.

Also it makes it very readable!

Keithin8a
  • 161