2
\$\begingroup\$

To avoid the overridable method call in constructor dilemma this approache delegate the variables initialization and calls to overridable methods to a member method that return reference to the instance its self and exception to be thrown if trying to use instance without invoking the initialization method/s so that the creation of objects will look like this:

SuperClass sup = new SuperClass().initialize("big");
SubClass sub = new SubClass().initialize("sato", "try initialzer");
SubClass sub2 = new SubClass().initialize("happy");
SubClass sub3 = new SubClass().initialize();

My super class should implement methods of initializable interface it has two methods:

  1. Initialize method in which I write code to do the following in order

    1. Call setter methods and other initialization code
    2. Make call to overridable method putting them at end of the method. This method can be overloaded to provide versions of it with necessary arguments as illustrated in complex example
  2. The isInitialzed method to check if the initialization done or not when a call to a method in my class that reference a variable that is initialized in initialize method and initialization not done, I throw my NonInitializedInstanceException.

The subclass should override the initialize method/s "all the super class versions of the initialize method" and do the following in order:

  1. Initialize its variables
  2. Can call super method that don't throw NonInitializedInstanceException
  3. Call super.initialize() method
  4. Call setter methods "note that the super call the setters so to avoid our setter been useless we should call it after super initialize method call"
  5. Can call super method that throw NonInitializedInstanceException

Handling overloaded versions of initialize done by calling the one with more parameters from the one with fewer parameters.

The interface:

 /**
 * @author ahmed mazher
 * @param <T> making class initializable 
 * 
 */
public interface Initializable<T> {

    /**
     * do the initialization job and return reference to the implementing
     * class current instance 
     * @return
     */
    T initialize();

    /**
     * test the variables that you initialize in the initialize method
     * if it was initialized or not "or what ever test you see appropriate"
     * @return
     */
    boolean isInitialized();
}

The NonInitializedInstanceException class:

/**
 * @author Ahmed mazher 
 * runtime exception to be thrown by method that reference variables that
 * are initialized by the initialize method of the initializable interface
 * you can test if initialization was done or not by isInialized method 
 * of the same interface if false throw the exception 
 */
public class NonInitializedInstanceException extends RuntimeException {

    private static final long serialVersionUID = 1L;


    @Override
    public String getMessage() {
        return super.getMessage() 
                + " you must call instance.initialize() method "
                + "before calling this method";
    }

}

Example of using them:

/**
 * @author ahmed mazher
 * testing the initializer pattern
 */
class LearnJava {

    public static void main(String args[]) {
        SuperClass sup = new SuperClass().initialize();
        SubClass sub = new SubClass().initialize();
    }

}

class SuperClass implements Initializable<SuperClass> {

    private String initializeDependentName;
    private final String nonInitializeDependentVal = "well done";
    @Override
    public SuperClass initialize() {
        //initializing super Class fields
        setInitializeDependentName("super class");
        //overidable method call in initializer "replacement of 
        //call in constructor"
        overridablePrintName();
        return this;
    }

    public final String getInitializeDependentName() throws NonInitializedInstanceException {
        if (!isInitialized()) {
            throw new NonInitializedInstanceException();
        }
        return initializeDependentName;
    }

    public final void setInitializeDependentName(String initializeDependentName) {
        this.initializeDependentName = initializeDependentName;
    }

    public final void youCanCallMeBeforeInitialization(){
        System.out.println(nonInitializeDependentVal);
    }

    public void overridablePrintName() {
        System.out.println("I'm the super method and my name is " + getInitializeDependentName());
    }

    @Override
    public boolean isInitialized() {
        return initializeDependentName != null;
    }
//to avoid some one messing with isInitialized() make your
    //private one and use it instead but don't forget to call it inside
    //the body of isInitialized() to maintian integrity of inheritance hierarchy
//    private boolean privateIsInitialized(){
//        return initializeDependentName != null;
//    }
//    @Override
//    public boolean isInitialized() {
//        return privateIsInitialized();
//    }

}

class SubClass extends SuperClass {

    private String job;

    @Override
    public SubClass initialize() {
        //initializing subClass fields
        setJob("testing initialize method");
        //call super method that throws nonInitialiezinstanceException 
        //before call super.initialize() method will rise the exception
        //unless you overided the isInitialized method "welling to take the risk"
        //------------------------------------------------------------------
        //System.out.println(getName().toUpperCase());
        //------------------------------------------------------------------
        //call super method that not throws nonInitialiezinstanceException 
        //before call super.initialize() method
        youCanCallMeBeforeInitialization();
        setInitializeDependentName("subClass");
        //calling super.initialize() method initialize super methods
        super.initialize();//initialize the super class
        //call super method that throws nonInitialiezinstanceException 
        //after call super.initialize() method will not rise the exception
        //unless you overided the isInitialized method in stupid way ^_^
        System.out.println(getInitializeDependentName().toUpperCase());
        //calling the setter method now to change my property as calling
        //it before the super initialization is useless as the super 
        //initialization will erase it
        setInitializeDependentName("subClass");
        System.out.println(getInitializeDependentName().toUpperCase());
        return this;
    }

    public final void setJob(String job) {
        this.job = job;
    }

    @Override
    public void overridablePrintName() {
        System.out.println("i'm overriden method and i do " + job);
    }

    @Override
    public boolean isInitialized() {
        //return false;//stupid version 
        //return true;//take the risk
        return job != null && super.isInitialized();//a good one 
    }

}

A more complicated example using initializer with arguments:

class LearnJava {

    public static void main(String args[]) {
        SuperClass sup = new SuperClass().initialize("big");
        SubClass sub = new SubClass().initialize("sato", "try initializer");
        SubClass sub2 = new SubClass().initialize("happy");
        SubClass sub3 = new SubClass().initialize();
    }

}

class SuperClass implements Initializable<SuperClass> {

    private String initializeDependentName;
    private final String nonInitializeDependentVal = "well done";

    @Override
    public SuperClass initialize() {
        return initialize("i'm super with no name");
    }

    public SuperClass initialize(String name) {
        //initializing super Class fields
        setInitializeDependentName(name);
        //overidable method call in initializer "replacement of 
        //call in constructor"
        overridablePrintName();
        return this;
    }

    public final String getInitializeDependentName() throws NonInitializedInstanceException {
        if (!isInitialized()) {
            throw new NonInitializedInstanceException();
        }
        return initializeDependentName;
    }

    public final void setInitializeDependentName(String initializeDependentName) {
        this.initializeDependentName = initializeDependentName;
    }

    public final void youCanCallMeBeforeInitialization() {
        System.out.println(nonInitializeDependentVal);
    }

    public void overridablePrintName() {
        System.out.println("i'm the super method my name is " + getInitializeDependentName());
    }

    @Override
    public boolean isInitialized() {
        return initializeDependentName != null;
    }

    //to avoid some one messing with isInitialized() make your
    //private one and use it instead but don't forget to call it inside
    //the body of isInitialized() to maintian integrity of inheritance hierarchy
//    private boolean privateIsInitialized(){
//        return initializeDependentName != null;
//    }
//    @Override
//    public boolean isInitialized() {
//        return privateIsInitialized();
//    }
}

class SubClass extends SuperClass {

    private String job;

    @Override
    public SubClass initialize() {
        return initialize("i'm subclass no one give me name");
    }

    @Override
    public SubClass initialize(String name) {
        return initialize(name, "no one give me job");
    }

    public SubClass initialize(String name, String job) {
        //initializing subClass fields
        setJob(job);
        //call super method that throws nonInitialiezinstanceException 
        //before call super.initialize() method will rise the exception
        //unless you overided the isInitialized method "welling to take the risk"
        //------------------------------------------------------------------
        //System.out.println(getName().toUpperCase());
        //------------------------------------------------------------------
        //call super method that not throws nonInitialiezinstanceException 
        //before call super.initialize() method
        youCanCallMeBeforeInitialization();
        //calling super.initialize() method initialize super methods
        super.initialize(name);//initialize the super class
        //call super method that throws nonInitialiezinstanceException 
        //after call super.initialize() method will not rise the exception
        //unless you overided the isInitialized method in stuped way ^_^
        System.out.println(getInitializeDependentName().toUpperCase());
        //calling the setter method now to change my property as calling
        //it before the super initialization is useless as the super 
        //initialization will erase it
        setInitializeDependentName("setter called in subclass");
        System.out.println(getInitializeDependentName().toUpperCase());
        return this;
    }

    public final void setJob(String job) {
        this.job = job;
    }

    @Override
    public void overridablePrintName() {
        System.out.println("i'm overiden method my name is "
                + getInitializeDependentName()
                + " and i do " + job);
    }

    @Override
    public boolean isInitialized() {
        //return false;//stuped version 
        //return true;//take the risk
        return job != null && super.isInitialized();//a good one 
    }

}

Suggestion of using static method factory pattern. As I expected, static pain never ends. Here is my trial to adapt my pattern into static one. I failed, so I don't know how to make subClass invoke superClass and create a method to affect its instance.

class LearnJava {

    public static void main(String args[]) {
        SuperClass.create("nono");
        SubClass.create("sato","test factory");

    }

}

class SuperClass{

    private String name;
    protected SuperClass() {
    }

    public static SuperClass create(String name) {
        SuperClass sup = new SuperClass();
        //initializing super Class fields
        sup.setName(name);
        //overidable method call useless as i always call the super instance version 
        sup.overridablePrintName();
        return sup;
    }

    public final String getName(){
        return name;
    }

    public final void setName(String name) {
        this.name = name;
    }

    public void overridablePrintName() {
        System.out.println("i'm the super method my name is " + getName());
    }
}

class SubClass extends SuperClass {

    private String job;

    protected SubClass() {
        super();
    }

    public static SubClass create(String name, String job) {
        SubClass sub = new SubClass();
        //initializing subClass fields
        sub.setJob(job);
        //call the super class initializer to affect this sub class instatnce 
        //seems no way to do it
        sub.create(name);
        SubClass.create(name);
        SuperClass.create(name);
        return sub;
    }

    public final void setJob(String job) {
        this.job = job;
    }

    @Override
    public void overridablePrintName() {
        System.out.println("i'm overiden method my name is "
                + getName()
                + " and i do " + job);
    }
}
\$\endgroup\$
  • \$\begingroup\$ What exactly do you want to get reviewed? The usage? \$\endgroup\$ – Pimgd Jul 24 '16 at 14:59
  • \$\begingroup\$ yes, is my concept correct, bug safe, complete or not, recommendation to improve it \$\endgroup\$ – Ahmed Mazher Jul 24 '16 at 16:56
  • \$\begingroup\$ also are their a better alternative, it tried the classic static factory but it has the limitation i mentioned above \$\endgroup\$ – Ahmed Mazher Jul 24 '16 at 16:59
1
\$\begingroup\$

Initial code review

  • You declared your NonInitializedInstanceException as RuntimeException and later declared your methods to throw this unchecked exception. Either change the excetpion type to Exception or remove the throws 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 suitable
  • Based 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 or overridablePrintName. 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.

\$\endgroup\$
  • \$\begingroup\$ "But as the child is already initialized in a Java sense" yes but problem arise from the the actual initialization of fields e.g referencing a null object division by zero , you can make the constructor do the task of initializing the fields but i chosen to delegate every thing to the initialize method \$\endgroup\$ – Ahmed Mazher Aug 2 '16 at 0:03
  • \$\begingroup\$ sorry i have some problem with editing \$\endgroup\$ – Ahmed Mazher Aug 2 '16 at 0:04
  • \$\begingroup\$ i think the whole could be simple avoided if the restriction that the super key word must be the first statement in the child constructor be lessened to super key word must come before any method invocation "allowing initialization of child fields" public SubClass(int someValue){ this.someValue = someValue; super() } \$\endgroup\$ – Ahmed Mazher Aug 2 '16 at 0:13

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.