96

I have some big (more than 3 fields) objects that can and should be immutable. Every time I run into that case I tend to create constructor abominations with long parameter lists.

It doesn't feel right, it is hard to use, and readability suffers.

It is even worse if the fields are some sort of collection type like lists. A simple addSibling(S s) would ease the object creation so much but renders the object mutable.

What do you guys use in such cases?

I'm on Scala and Java, but I think the problem is language agnostic as long as the language is object oriented.

Solutions I can think of:

  1. "Constructor abominations with long parameter lists"
  2. The Builder Pattern
Russia Must Remove Putin
  • 374,368
  • 89
  • 403
  • 331
Malax
  • 9,436
  • 9
  • 48
  • 64
  • 5
    I think the builder pattern is the best and most standard solution to this. – Zachary Wright May 17 '10 at 12:27
  • @Zachary: What you are advocating only works for a special form of the builder pattern, as explained here by Joshua Bloch: http://www.drdobbs.com/java/208403883?pgno=2 I prefer to go with the related "fluent interface" term to call this "special form of the builder pattern" (see my answer). – SyntaxT3rr0r May 17 '10 at 13:45

9 Answers9

76

Well, you want both an easier to read and immutable object once created?

I think a fluent interface CORRECTLY DONE would help you.

It would look like this (purely made up example):

final Foo immutable = FooFactory.create()
    .whereRangeConstraintsAre(100,300)
    .withColor(Color.BLUE)
    .withArea(234)
    .withInterspacing(12)
    .build();

I wrote "CORRECTLY DONE" in bold because most Java programmers get fluent interfaces wrong and pollute their object with the method necessary to build the object, which is of course completely wrong.

The trick is that only the build() method actually creates a Foo (hence you Foo can be immutable).

FooFactory.create(), whereXXX(..) and withXXX(..) all create "something else".

That something else may be a FooFactory, here's one way to do it....

You FooFactory would look like this:

// Notice the private FooFactory constructor
private FooFactory() {
}

public static FooFactory create() {
    return new FooFactory();
}

public FooFactory withColor( final Color col ) {
    this.color = color;
    return this;
}

public Foo build() {
    return new FooImpl( color, and, all, the, other, parameters, go, here );
}
Phil
  • 6,561
  • 4
  • 44
  • 69
SyntaxT3rr0r
  • 27,745
  • 21
  • 87
  • 120
  • 11
    @ all : please no complaint about the "Impl" postfix in "FooImpl": this class is hidden inside a factory and no one will ever see it besides the person writing the fluent interface. All the user cares about is that he gets a "Foo". I could have called "FooImpl" "FooPointlessNitpick" as well ;) – SyntaxT3rr0r May 17 '10 at 13:34
  • 5
    Feeling pre-emptive? ;) You've been nitpicked in the past about this. :) – Greg D May 17 '10 at 14:35
  • Great answer, WizardOfOdds. I've seen this pattern successfully applied, and it really does meet the requirements expressed by the OP. – Timothy May 17 '10 at 14:45
  • @Greg D: actually I don't think people nitpicked *me about it :) I think it's even the first I post a piece of code with "Impl" :) But I've sure *read* a lot of discussion/holy-wars on that subject here on SO ;) – SyntaxT3rr0r May 17 '10 at 16:11
  • 1
    "I wrote "CORRECTLY DONE" in bold because most Java programmers get fluent interfaces wrong and pollute their object with the method necessary to build the object, which is of course completely wrong." What exactly do you mean by this? The example you posted is very close to the Builder pattern but using fluent naming. What is the common error? – pbh101 May 17 '10 at 23:25
  • 3
    I believe the common error he's referring to is that people add the "withXXX" (etc) methods *to* the `Foo` object, rather than having a separate `FooFactory`. – Dean Harding May 18 '10 at 00:57
  • 2
    @pbh101: what codeka commented: in a lot of the blog/SO questions you see people doing what they think are "fluent interface" by adding the method related to object construction in the classes you want to construct. But, yup, basically a fluent interface is a specific form of the builder pattern (in the article here Joshua Bloch talks about that "special form": drdobbs.com/java/208403883?pgno=2 ). But the accepted term nowadays for such a construct is "fluent interface", not "builder pattern" (even if a fluent interface is done using a specific form of the builder pattern). – SyntaxT3rr0r May 18 '10 at 21:10
  • @codeka: lets put it this way: every fluent interface in Java should be done using the Builder pattern but everytime the Builder pattern is used does not mean you're creating a "fluent interface". – SyntaxT3rr0r May 18 '10 at 21:11
  • my comment @ http://stackoverflow.com/questions/114342/what-are-code-smells-what-is-the-best-way-to-correct-them/114360#114360 – Behrooz Jul 13 '10 at 14:37
  • Your answer just triggered the "AHA!" I needed to understand why I've had issues when tinkering with fluent interfaces: it's the *factory* that's "fluent", not the object! Thanks. :-) – Rodney Gitzel Sep 01 '10 at 18:24
  • 4
    You still have the `FooImpl` constructor with 8 parameters. What is the improvement? – Mot Aug 08 '11 at 08:15
  • @MikeL. I realise this is an old comment, but for the sake of completeness: The code that wants the object created by the `FooFactory` doesn't have to ever see the `FooImpl` class - all it needs to see is the `Foo` interface that `FooImpl` conforms to. If you only need to set 3 parameters, the `FooFactory.build()` method could fill the `FooImpl` constructor parameters with defaults. – Edd Feb 15 '13 at 12:17
  • 4
    I wouldn't call this code `immutable`, and I would be afraid of people reusing factory objects because they think it is. I mean: `FooFactory people = FooFactory.create().withType("person"); Foo women = people.withGender("female").build(); Foo talls = people.tallerThan("180m").build();` where `talls` would now only contain women. This shouldn't happen with an immutable API. – Thomas Ahle Mar 19 '14 at 15:53
  • I don't see the problem having withXXX() on the class itself rather than a builder. Because I would sometimes like to take an existing object and 'modify' a field (of course returning a new copy). And I also have with() methods that are complicated and have lots of business logic, like `TextArea.withReplaceLine(String newLine, int lineNumber)` . I wouldn't want that logic to be in a builder class. – LegendLength Jul 16 '17 at 15:17
  • 1
    Even in 2017, still high-rank in "Related", so I'll add my comment on fluent interfaces. Advantage: readability. Disadvantage: needs complex logics to be as safe as the multi-args cconstructor. @Thomas Ahle already pointed out that it's too easy to keep unwanted previous settings. And you need to check whether all necessary parameters have been set before the build() call. So fluent interfaces are good if all the parameters are optional, but if not, I'd recommend a multi-arg constructor. – Ralf Kleberhoff Aug 11 '17 at 12:28
  • As Thomas Ahle points out, there is a critical mistake in this example: the `with` method *mutates the builder*; such a method would more correctly be prefixed `set`, and is harder to work with than an immutable version. A better implementation would be something like `FooFactory inst = this.clone(); inst.color = color; return inst;` – IMSoP Apr 26 '19 at 13:49
60

In Scala 2.8, you could use named and default parameters as well as the copy method on a case class. Here's some example code:

case class Person(name: String, age: Int, children: List[Person] = List()) {
  def addChild(p: Person) = copy(children = p :: this.children)
}

val parent = Person(name = "Bob", age = 55)
  .addChild(Person("Lisa", 23))
  .addChild(Person("Peter", 16))
Martin Odersky
  • 20,470
  • 9
  • 51
  • 49
  • 32
    +1 for inventing the Scala language. Yeah, it's an abuse of the reputation system but... aww... I love Scala so much i had to do it. :) – Malax May 17 '10 at 14:50
  • 1
    Oh, man... I have just answered something almost identical! Well, I'm in good company. :-) I wonder that I didn't see your answer before, though... – Daniel C. Sobral May 17 '10 at 15:47
20

Well, consider this on Scala 2.8:

case class Person(name: String, 
                  married: Boolean = false, 
                  espouse: Option[String] = None, 
                  children: Set[String] = Set.empty) {
  def marriedTo(whom: String) = this.copy(married = true, espouse = Some(whom))
  def addChild(whom: String) = this.copy(children = children + whom)
}

scala> Person("Joseph").marriedTo("Mary").addChild("Jesus")
res1: Person = Person(Joseph,true,Some(Mary),Set(Jesus))

This does have its share of problems, of course. For instance, try making espouse and Option[Person], and then getting two persons married to each other. I can't think of a way to solve that without resorting to either a private var and/or a private constructor plus a factory.

Daniel C. Sobral
  • 295,120
  • 86
  • 501
  • 681
11

Here are a couple of more options:

Option 1

Make the implementation itself mutable, but separate the interfaces that it exposes to mutable and immutable. This is taken from the Swing library design.

public interface Foo {
  X getX();
  Y getY();
}

public interface MutableFoo extends Foo {
  void setX(X x);
  void setY(Y y);
}

public class FooImpl implements MutableFoo {...}

public SomeClassThatUsesFoo {
  public Foo makeFoo(...) {
    MutableFoo ret = new MutableFoo...
    ret.setX(...);
    ret.setY(...);
    return ret; // As Foo, not MutableFoo
  }
}

Option 2

If your application contains a large but pre-defined set of immutable objects (e.g., configuration objects), you might consider using the Spring framework.

Little Bobby Tables
  • 5,261
  • 2
  • 39
  • 49
  • 3
    Option 1 is clever (but not *too* clever), so I like it. – Timothy May 17 '10 at 14:47
  • 4
    I did this earlier, but in my opinion it is far away from being a good solution because the object is still mutable, only the mutating methods are "hidden". Maybe i am too picky about this subject... – Malax May 17 '10 at 15:10
  • a variant on option 1 is to have separate classes for Immutable and Mutable variants. This provides better safety than the interface approach. Arguably you're lying to the consumer of the API whenever you give them a mutable object named Immutable that simply needs to be cast to it's mutable interface. The separate class approach requires methods to convert back and forth. The JodaTime API uses this pattern. See DateTime and MutableDateTime. –  Aug 12 '11 at 20:40
6

It helps to remember there are different kinds of immutability. For your case, I think "popsicle" immutability will work really well:

Popsicle immutability: is what I whimsically call a slight weakening of write-once immutability. One could imagine an object or a field which remained mutable for a little while during its initialization, and then got “frozen” forever. This kind of immutability is particularly useful for immutable objects which circularly reference each other, or immutable objects which have been serialized to disk and upon deserialization need to be “fluid” until the entire deserialization process is done, at which point all the objects may be frozen.

So you initialize your object, then set a "freeze" flag of some sort indicating that its no longer writable. Preferably, you'd hide the mutation behind a function so the function is still pure to clients consuming your API.

ParkerM
  • 302
  • 1
  • 4
  • 17
Juliet
  • 80,494
  • 45
  • 196
  • 228
  • 1
    Downvotes? Anyone want to leave a comment on why this isn't a good solution? – Juliet May 17 '10 at 23:21
  • +1. Maybe someone downvoted this because you are hinting at the use of `clone()` to derive new instances. – finnw Jul 21 '10 at 02:40
  • Or maybe it was because it can compromise thread safety in Java: http://java.sun.com/docs/books/jls/third_edition/html/memory.html#17.5 – finnw Jul 23 '10 at 13:37
  • A disadvantage is that it requires future developers to be careful to handle the "freeze" flag. If a mutator method is added later that forgets to assert that the method is unfrozen, there could be problems. Similarly, if a new constructor is written that should, but does not, call the `freeze()` method, things could get ugly. – stalepretzel Dec 25 '13 at 06:49
5

Consider four possibilities:

new Immutable(one, fish, two, fish, red, fish, blue, fish); /*1 */

params = new ImmutableParameters(); /*2 */
params.setType("fowl");
new Immutable(params);

factory = new ImmutableFactory(); /*3 */
factory.setType("fish");
factory.getInstance();

Immutable boringImmutable = new Immutable(); /* 4 */
Immutable lessBoring = boringImmutable.setType("vegetable");

To me, each of 2, 3, and 4 is adapted to a difference situation. The first one is hard to love, for the reasons cited by the OP, and is generally a symptom of a design that has suffered some creep and needs some refactoring.

What I'm listing as (2) is good when there is no state behind the 'factory', whereas (3) is the design of choice when there is state. I find myself using (2) rather than (3) when I don't want to worry about threads and synchronization, and I don't need to worry about amortizing some expensive setup over the production of many objects. (3), on the other hand, is called forth when real work goes into the construction of the factory (setting up from an SPI, reading configuration files, etc).

Finally, someone else's answer mentioned option (4), where you have lots of little immutable objects and the preferable pattern is to get news ones from old ones.

Note that I'm not a member of the 'pattern fan club' -- sure, some things are worth emulating, but it seems to me that they take on an unhelpful life of their own once people give them names and funny hats.

bmargulies
  • 97,814
  • 39
  • 186
  • 310
5

You could also make the immutable objects expose methods that look like mutators (like addSibling) but let them return a new instance. That's what the immutable Scala collections do.

The downside is that you might create more instances than necessary. It's also only applicable when there exist intermediate valid configurations (like some node without siblings which is ok in most cases) unless you don't want to deal with partially built objects.

For example a graph edge which has no destination yet isn't a valid graph edge.

ziggystar
  • 28,410
  • 9
  • 72
  • 124
  • The alleged downside - creating more instances than necessary - isn't really that much of a problem. Object allocation is very cheap, as is garbage collection of short-lived objects. When escape analysis is enabled by default, this kind of "intermediate objects" are likely to be stack allocated and cost literally nothing to create. – gustafc May 17 '10 at 13:56
  • 2
    @gustafc: Yep. Cliff Click once told the story of how they benchmarked Rich Hickey's Clojure Ant Colony simulation on one of their big boxes (864 cores, 768 GB RAM): 700 parallel threads running on 700 cores, each one at 100%, generating over 20 GB of ephemeral garbage *per second*. The GC didn't even break a sweat. – Jörg W Mittag May 17 '10 at 14:11
4

Another potential option is to refactor to have fewer configurable fields. If groups of fields only work (mostly) with each other, gather them up into their own small immutable object. That "small" object's constructors/builders should be more manageable, as will the constructor/builder for this "big" object.

Carl
  • 7,538
  • 1
  • 40
  • 64
  • 1
    note: mileage for this answer may vary with problem, code-base, and developer skill. – Carl May 17 '10 at 12:58
2

I use C#, and these are my approaches. Consider:

class Foo
{
    // private fields only to be written inside a constructor
    private readonly int i;
    private readonly string s;
    private readonly Bar b;

    // public getter properties
    public int I { get { return i; } }
    // etc.
}

Option 1. Constructor with optional parameters

public Foo(int i = 0, string s = "bla", Bar b = null)
{
    this.i = i;
    this.s = s;
    this.b = b;
}

Used as e.g. new Foo(5, b: new Bar(whatever)). Not for Java or C# versions before 4.0. but still worth showing, as it's an example how not all solutions are language agnostic.

Option 2. Constructor taking a single parameter object

public Foo(FooParameters parameters)
{
    this.i = parameters.I;
    // etc.
}

class FooParameters
{
    // public properties with automatically generated private backing fields
    public int I { get; set; }
    public string S { get; set; }
    public Bar B { get; set; }

    // All properties are public, so we don't need a full constructor.
    // For convenience, you could include some commonly used initialization
    // patterns as additional constructors.
    public FooParameters() { }
}

Usage example:

FooParameters fp = new FooParameters();
fp.I = 5;
fp.S = "bla";
fp.B = new Bar();
Foo f = new Foo(fp);`

C# from 3.0 on makes this more elegant with object initializer syntax (semantically equivalent to the previous example):

FooParameters fp = new FooParameters { I = 5, S = "bla", B = new Bar() };
Foo f = new Foo(fp);

Option 3:
Redesign your class not to need such a huge number of parameters. You could split its repsonsibilities into multiple classes. Or pass parameters not to the constructor but only to specific methods, on demand. Not always viable, but when it is, it's worth doing.

Joren
  • 14,472
  • 3
  • 50
  • 54