Initial code review
You declared your
NonInitializedInstanceException
asRuntimeException
and later declared your methods to throw this unchecked exception. Either change the excetpion type toException
or remove thethrows NonInitializedInstanceException
from the methods.I'm not sure how a class can be dependent on its name. If you meant that it needs to be specified
requiredName
would probably more suitableBased on the API itself it is not clear if a method may be invoked while initialization or not. I doubt that a real business case would include a naming like
youCanCallMeBeforeInitialization
oroverridablePrintName
. Therefore a documentation overhead is mandatory which leads to an API that might be harder and more discouraging to use.I'm also not fully sure if at the given initialization time preventing overriden method invocations are necessary as first the super class constructor is executed, which does nothing, followed by the child constructor, which also does nothing. In a Java sense the object is now fully initialized. Afterwards the childs
initialize
method is invoked which invokes the super initialization method at some point. But as the child is already initialized in a Java sense, invoking any other method within the initializer method is admissible. An overriden method call within the initialization method will thus always return the correct overriden state.
Problem description
The actual problem you try to solve is to set state within the constructor which may not yet be available as the parent class might invoke the overridable method, but as the child is not yet initialized, the overridden method does not return what may be assumed to be returned leaving the currently initialized object in an inconsistent state.
Your code initializes a new object and afterwards invokes initialize
which is basically a factory method which is similar to a builder but instead of returning a product reference it returns the reference of itself. As you already have a valid object through instantiating via new SuperClass()
throwing a NonInitializedInstanceException
may prevent further usage of the object, though overridable methods will always set the correct value as the object was already created.
Alternative approaches
As you asked for alternative approaches, I'll try to depict two other ways to fix the overridable method in constructor issue:
- Parameter object
- Builder pattern
Parameter object
In Java the constructor is usually used to initialize the object state. Delaying the initialization to a further method is possible but only useful in certain circumstances like de/serialization or some lazy initializations IMO.
If you don't need lazy initialization you may be better off with a parameter object, which can be a simple map or a specialized parameter class.
public class Sample {
protected final SomeType someField;
protected SomeOtherType someOtherField;
public Sample(Map<String, Object> parameters) {
if (parameters != null) {
if (parameters.contains(KEY) {
this.someField = (SomeType)parameters.get(KEY);
} else if (parameters.contains(OTHER_KEY)) {
this.someOtherField = (SomeOtherType)parameters.get(OTHER_KEY);
}
} else {
throw new IllegalArgumentException("Could not initialize field 'someField'!");
}
}
}
An inheriting class could look like this:
public class SomeSample extends Sample {
private final YetSomeOtherType yetSomeOtherField;
public SomeSample(Map<String, Object> parameters) {
super(parameters);
if (parameters.contains(YET_OTHER_KEY)) {
this.yetSomeOtherField = (YetSomeOtherType)parameters.get(YET_OTHER_KEY);
this.someOtherField = null; // admissible
} else {
throw new IllegalArgumentException("Could not initialize field 'someOtherField'!");
}
}
}
Here, first the fields of the parent are initialized and for none final fields, the may even be updated like with someOtherField
in the example below.
This approach also allows to set arguments via reflection easily. A sample method which could be put into the parent class may look like the one below:
protected final void setRequiredField(Field field, Map<String, Object> parameters, String key) {
if (null == field) {
throw new InvalidArgumentException("Invalid field to inject value into for key '"
+ key + "' provided");
}
if (null == key || "".equals(key)) {
throw new InvalidArgumentException("Could not inject value into field '"
+ field.getName() + "' as key was invalid");
}
if (null == parameters || parameters.isEmpty()) {
throw new InvalidArgumentException("Invalid parameter object provided. Could not inject value with key '"
+ key + "' into field '" + field.getName() + "'!");
}
if (parameters.contains(key)) {
field.set(this, parameters.get(key));
} else {
throw new IllegalArgumentException("Could not set value for field '"
+ field.getName() + "' as provided parameters did not contain key: '"
+ key + "'");
}
}
In the respective constructor you then need to change the code to something like:
public Sample(Map<String, Object> parameters) {
setRequiredField(getClass().getDeclaredField("someField"), parameters, KEY);
}
...
public SomeSample(Map<String, Object> parameters) {
super(parameters);
setRequiredField(getClass().getDeclaredField("someOtherField"), parameters, OTHER_KEY);
}
Instead of a map you can also use a custom parameter object which contains respective getters and setters. Note that I don't handle all kinds of exceptions here as this should only be a short tutorial on parameter objects rather then reflection.
This approach either sets the field value directly or invokes a final and therefore none overridable setter method, which also can be reused by children.
Builder pattern
According to a well-accepted answer the invocation of overridable methods within the constructor can easily get fixed using a builder pattern.
Compared to generic parameter maps, builders allow for compile time safetiness, especially if the parameter object is bound to objects rather then more specific types. In addition to that, reflection also has its price and if you create objects often, this may slow down the application notably.
Builders may use certain default values for fields to set and only overwrite those that are explicitly set. Also, builders usually create only fully initialized instances. Builders are especially useful if you have objects which allow to inject multiple values within the constructor. Besides minimizing the overhead of remembering all kinds of constructors they also allow to group certain parameters logically. Consider some shape builder for instance. you could do something like withPosX(x).withPosY(y).withColor(yellow)...
though, logically more expressive would be to use something like withPosition(x, y).withColor(yellow)...
. Modern Java editors also provide code completion support for builder setters and therefore provide a fluent interface for setting needed parameters. Last but not least, on using builders you usually can assign the values upon build-time to final fields and therefore prevent reassigning other references to this field.
While builders are not used often in combination with inheritance, it is possible with some overhead though. I tend to keep necessary parameters which have no default values within the constructor of the builder and optional parameters or ones which value can be overwritten as typical builder setter methods.
The following sample code presents a scenario where a base builder is extended by child builders. The code is also available on Github. A basic builder has a form like this:
public abstract class Base<Z> {
public static abstract class Builder<B extends Builder<? extends B, Z>, Z> {
// necessary parameters
protected final String a;
protected final String b;
protected final String c;
// optional parameters
protected Z z = null;
public Builder(String a, String b, String c) {
this.a = a;
this.b = b;
this.c = c;
}
@SuppressWarnings("unchecked")
public B withOptionalZ(Z z) {
this.z = z;
return (B)this;
}
public abstract <T> T build();
}
protected final String name;
protected final String a;
protected final String b;
protected final String c;
protected Z z = null;
protected Base(String name, String a, String b, String c) {
this(name, a, b, c, null);
}
protected Base(String name, String a, String b, String c, Z z) {
this.name = name;
this.a = a;
this.b = b;
this.c = c;
this.z = z;
}
public String getA() {
return a;
}
public String getB() {
return b;
}
public String getC() {
return c;
}
protected abstract String getContent();
@Override
public String toString() {
return name+"[A: " + a + ", B: " + b + ", C: " + c
+ (z != null ? ", Z: " + z.toString() : "") + getContent() +"]";
}
}
Z
is the generic type of the optional parameter whereas B
is the generic type of the actual builder. In order to invoke the concrete builder on child builders rather then the base builder the generic type is passed as return value on the builder setters. Specifying the generated type the builder produces via the generic type would make sense, though in the current version of Java (Java 7 and 8) on using the diamond opperator, Java is incapable of resolving the correct types. To bypass this limitation instead of returning some type T of the form ? extends Param<Z>
the return type is based on the type the builder product is assigned to via <T> T
.
The genericReturnType
branch in the Github repository also showcases how to solve this issue, though instead of a simple diamond operator a quite complex generic definition has to be provided in order to execute the tests without editor warnings.
A product and builder extension do look like this:
public class SampleA<D,E,Z> extends Base<Z> {
@SuppressWarnings("unchecked")
public static class Builder<B extends SampleA.Builder<? extends B, D, E, Z>,
D,E,Z>
extends Base.Builder<SampleA.Builder<B,D,E,Z>, Z> {
protected D d;
protected E e;
public Builder(String a, String b, String c) {
super(a, b, c);
}
public B withD(D d) {
this.d = d;
return (B)this;
}
public B withE(E e) {
this.e = e;
return (B)this;
}
@Override
public <T> T build() {
return (T) new SampleA<>("Test A", a, b, c, z, d, e);
}
}
protected final D d;
protected final E e;
protected SampleA(String name, String a, String b, String c, Z z, D d, E e) {
super(name, a, b, c, z);
this.d = d;
this.e = e;
}
public D getD() {
return d;
}
public E getE() {
return e;
}
@Override
protected String getContent() {
return ", D: " + d + ", E: " + e;
}
}
D
, E
, F
and G
in this sample are just simple classes of the form:
public class D {
@Override
public String toString() {
return "D";
}
}
to provide some test output and showcase the functionality of the pattern. SampleB
is just a ripoff of SampleA
which defines setters for F
and G
rather then D
and E
.
The code below is just a simple showcase scenario:
public class Main
{
public static void main(String ... args)
{
// sample builder invocation
SampleA<D,E,?> a = new SampleA.Builder<>("a","b","c")
.withD(new D()).withE(new E()).build();
// sample with invoking setter for optional parameter
SampleB<F,G,String> b = new SampleB.Builder<>("a","b","c")
.withF(new F()).withG(new G()).withOptionalZ("z").build();
// assigning the product of the builder to a parent type
Base<String> c = new SampleA.Builder<>("a","b","c")
.withD(new D()).withE(new E()).withOptionalZ("z").build();
// omitted generic type by intention
Base d = new SampleB.Builder<>("a","b","c")
.withF(new F()).withG(new G()).build();
test(a);
test(b);
test(c);
test(d);
test(new SampleA.Builder<>("a","b","c")
.withD(new D()).withE(new E()));
test(new SampleB.Builder<>("a","b","c")
.withF(new F()).withG(new G()).withOptionalZ("z"));
testBase(new SampleA.Builder<>("a","b","c")
.withD(new D()).withE(new E()).withOptionalZ("z"));
testBase(new SampleB.Builder<>("a","b","c")
.withF(new F()).withG(new G()));
}
public static void test(SampleA<?,?,?> sample)
{
System.out.println("Test for SampleA: " + sample.toString());
}
public static void test(SampleB<?,?,?> sample)
{
System.out.println("Test for SampleB: " + sample.toString());
}
public static void test(Base<?> base)
{
System.out.println("Test for Base: " + base.toString());
}
public static void test(SampleA.Builder<?,?,?,?> builder)
{
System.out.println("Test for BuilderA: " + builder.build().toString());
}
public static void test(SampleB.Builder<?,?,?,?> builder)
{
System.out.println("Test for BuilderB: " + builder.build().toString());
}
public static void testBase(Base.Builder<?,?> builder)
{
System.out.println("Test for Base builder: " + builder.build().toString());
}
}
Eclipse and IntelliJ act differently on this code but both editors (as well as compiling and executing the code manually) produce the same output.
The code above should produce the following output:
Test for SampleA: Sample A[A: a, B: b, C: c, D: D, E: E]
Test for SampleB: Sample B[A: a, B: b, C: c, Z: z, F: F, G: G]
Test for Base: Sample A[A: a, B: b, C: c, Z: z, D: D, E: E]
Test for Base: Sample B[A: a, B: b, C: c, F: F, G: G]
Test for BuilderA: Sample A[A: a, B: b, C: c, D: D, E: E]
Test for BuilderB: Sample B[A: a, B: b, C: c, Z: z, F: F, G: G]
Test for Base builder: Sample A[A: a, B: b, C: c, Z: z, D: D, E: E]
Test for Base builder: Sample B[A: a, B: b, C: c, F: F, G: G]
While builder have a couple of advantages, the almost duplicate code requirement is for sure an overhead and therefore may be discouraging to use, especially for objects that do not come with a lot of parameters. Some also argue that the setters that need to be invoked to pass values to the object, which may result in a long chain of invocations, leads to code that is hard to read and therefore is also discouraging. With proper code formatting though I find the code often more readabl compared to invoking setters on the real object.
Certain setters (Product and Builder) may be replaced by Lombok annotations which may help to clean the code further and thus reduce the number of actual lines of code.
- 441
- 4
- 10