Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Preventing "this" escaping during construction with final fields

public class App {
    private final A a;
    private final Server server; 
    public App(){
        a = new A(this); //Bad, this is escaping before it's initialized.
    }
    @Subscribe //This event fires some time after App is finished constructing.
    public void registerStuff(RegisterEvent event){
        server = event.getServer(); //Not possible due to final field and this not being the constructor, is there such thing as a lazy final?
        a.register();
    }
}

public class A {
    private final App app;
    private final BuilderSpec spec;
    public A(App app){
        this.app = app;
        this.spec = app.getServer().builder(this.app).build();
    }
    public void register(){
        app.getServer().doStuff(this.app, this.spec);
    }
}

I've read a little about what "this" escaping is and realize that the previous code is bad, as I have little idea what the external processes are doing with the this reference, so it shouldn't be passed outside the constructor until it is constructed.

However, due to the final fields in both App and A, I really don't see how I can initialize this afterwards, or lazily. Is making the fields final less important then the this escaping? I'm not sure.

like image 673
Ryan Leach Avatar asked Jun 12 '15 15:06

Ryan Leach


4 Answers

Constructor dependencies can become a problem if they get cycled. Most of the IoC containers operate with post-construct methods to avoid problems with object instantiation.

Applying the principal to your code, it would look like

class App {
    private A a;
    public App() { 
      // not necessary to have, just to show that we have no logic inside 
    }
    protected void init() {  // choose access modifiers
        // init something in App;
        a = new A(this);  // safe to pass this now.           
    }
}

class A {
   private final App app;

   public A(App app) {
      this.app = app;
      // do whatever you need
      // if logic affects other components, create init() method here
   }      
}

Yes, A reference in App is not final. But this design is more natural and less error-prone.

like image 161
AdamSkywalker Avatar answered Oct 06 '22 00:10

AdamSkywalker


Do you even need an App class? A only appears to need a Server. Just move registerStuff() to from App to A and only use A.

public class A{



@Subscribe
public void registerStuff(RegisterEvent event){
    Server server = event.getServer();
    BuilderSpec spec = server.builder(this) // not sure what `builder()` needs but might only be A ?
                              .build();

    server.doStuff(this, spec);
}
 }
like image 23
dkatzel Avatar answered Oct 05 '22 22:10

dkatzel


Your code does not compile since you don't initialize the Server server variable which is final. Anyway you can pass a Provider which has the job of providing the App to your A when it becomes available (fully initialized). Your code can look like this:

public App(){
   appProvider = new Provider<App>()
    {
        private volatile App app;

        // setter here

        @Override
        public String get()
        {
            return app;
        }
    };
    a = new A(appProvider);

}

@Subscribe
public void registerStuff(RegisterEvent event){
    // set server to your App
    appProvider.set(this);
}

then you can check in A whether the provider's get returns null or not. I believe that the javax.inject package contains an appropriate Provider interface for you.

The registerStuff method looks good for providing the App since at that point it is fully initialized. This also answers your question about lazy initialization.

like image 24
Adam Arold Avatar answered Oct 05 '22 23:10

Adam Arold


As @Boris_the_Spider mentioned, passing this in the constructor is not necessarily a bad thing if you make sure your class is in a consistent state (see Passing “this” in java constructor). However, be careful of extending App in the future because in this case things can go very wrong (see Don't publish the "this" reference during construction)

However, another valid option (as you mentioned) would be to remove final from A. In this case, I would recommend declaring a getter method. Because A is private, it will only be accessed by class App, and so you just need to make sure App uses the getA() method whenever you need access to A:

public class App {
    private A a;
    private final Server server; 
    public App(){}

    // Define getter
    private A getA(){
        return a == null ? new A(this) : a;
    }

    @Subscribe
    public void registerStuff(RegisterEvent event){
        server = event.getServer();

        // Use getter
        getA().register();
    }
}
like image 43
bcorso Avatar answered Oct 05 '22 23:10

bcorso