Is a large static initializer a code smell?

Question

I am extending SimpleExpandableListAdapter in Android. I don't think Android's adapter is implemented very well, in that its constructors have a large number of rather complicated arguments and it has no setters or builder. In my class, most of those arguments are not dependent on the calling class so I want to build them internally. However, the arguments are nested Lists and arrays of ints and strings that need to be built programmatically.

Since nothing can be called before the super constructor, and instance methods can not be called before the super call returns, I currently have several static methods that I call from the super call:

super(getContext(), initGroupData(), groupLayout, initGroupFrom(), initGroupTo(),
        initChildData(), childLayout, initChildFrom(), initChildTo());

I see three ways of handling this: calling static methods like I am now, having a large static initializer that probably calls these same methods to initialize static variables that are then used in the super call, or encapsulating all of these methods into a builder.

I think right now I'm leaning towards the builder, but I'm wondering if there's any better way to handle this.

Answer 1

Having static helper functions to create the constructor argument is a perfectly sane solution, but these functions are limited in what operations they can perform since they must produce exactly one argument each, and cannot communicate with each other.

In the most general case where you want to adapt an interface Constructor(A, B, C) to a more usable interface Constructor(X, Y), you can define a private helper constructor that takes a private ArgumentObject and chains to the existing constructor. The more usable constructor then chains to the helper constructor via a static helper function to create the argument object:

class Constructor {
  // constructor you want to wrap
  public Constructor(A a, B b, C c) { ... }
  // better constructor you are defining
  public Constructor(X x, Y y) { this(createArgumentObject(x, y)); }
  // helper constructor using an ArgumentObject
  private Constructor(ArgumentObject ao) { this(ao.a, ao.b, ao.c); }
  // helper to create the argument object
  private static ArgumentObject createArgumentObject(X x, Y y) { ... }
  private static class ArgumentObject { ... }
}

In languages that don't have constructor chaining within the same class (such as C++03), you would have to define an intermediate subclass for the helper constructor.

However, this technique is merely a generalization of your usage of static functions in the constructor arguments. The other solutions you have proposed have various drawbacks which is why I would avoid them unless there is a very good reason to prefer them:

• implementing good builders takes a lot of effort for very little value. If the new constructor is simple enough, you can do without a builder. Assuming the class you are wrapping performs solid validation, you could make the argument object public so that arguments can be passed using the Constructor(new Constructor.Arguments {{ foo = 42; bar = baz; }}) idiom.

• using static variables that are computed in a static initializer block makes sense for truly static data, but care must be taken to avoid global state. Unless the initialization is very simple, you should be using static functions to initialize these variables in order for the initialization to be testable. Now, the only advantage over directly using static methods is that the values are only computed once and reused for all initializations.

  Since your question indicates these initializations could be more complex, using static initializer blocks is a big no-no if testability is important for you. (If it isn't, you have more urgent problems.)