views:

163

answers:

7

Can I do the following?

public Manager(String userName) {
    game = new Game(userName);
    game.addManager(this);
}

The problem is that I refer to an object (this) in its constructor (before it was actually created).

+2  A: 

Yup its perfectly legal in Java, however not recommended. See here for more details in the this keyword.

James
While that technique is valid in Java, it's pretty dangerous.
Joachim Sauer
Perfectly valid?? It just compiles and nothing more.
Roman
I think this answer would be better if it said "its perfectly legal Java" which it is. But as in life, just because it is legal doesn't make it a good idea.
Yishai
Yeah I will update my words, posted it quite quick! What I was referring to was it *is* legal, not whether the OP should do it or not.
James
+3  A: 

Yes, you can do that, but you should not do that.

The problem is that publishing this while the constructor is still running can produce all kinds of strange side-effects, because some common guarantees don't hold true while the constructor is still running (for example final variables can seem to change their value while the constructor still runs).

This IBM developerWorks article describes the precautions to take when constructing objects and the reasoning behind those precautions. While the article discusses the subject in the light of multi-threading, you can have similar problems in a single-threaded environment when unknown/untrusted code gets a reference to this during construction.

(that last paragraph was "stolen" from one of my earlier answers).

Joachim Sauer
+1  A: 

As @James stated, you can, but it is not necessarily something you want to do. If game.addManager tries to access certain properties of the Manager, you can end up trying to access the properties of a Manager that haven't yet been initialized. A better approach is to have an external object call some init method (or some lifecycle method) to add the manager and not do it in the constructor.

Jeff Storey
A: 

see if this helps,it is actually for c/c++ but i think its the same for java:

http://www.gotw.ca/publications/mill13.htm

lajoo
A: 

This technique violates one of java concurrency concepts - safe publication. You should use init() method for this purpose or another technique.

You see, you can initialize some final fields in your constructor (or do some other initialization) after this reference have been escaped. If you pass instance of your object in its constructor to another object then you can get callback during construction. And it can cause inconsistent behavior, NPEs, dead-locks and so on.

Roman
+3  A: 

Although it is legal Java, and in the case you describe (where it is the last line of the constructor) it is a pretty safe thing to do (with certain edge cases exempted), as a practice it is a bad thing to do, and like using goto (in languages that support the keyword) it should be something you think long and hard about. For your case, a better practice would be to make the constructor private, remove the call to addManager and expose a static factory method:

 public static Manager createManager(String userName) {
        Manager manager = new Manager(userName);
        manager.game.addManager(manager);
        return manager;
 }

I should also point out that that kind of interdependency among classes (the manager knows about the game and the game knows about the manager) is definitely a code smell, and I would be as concerned about the need for that as I would be about passing this from the constructor.

Yishai
+1 for the interdependency code smell
sleske
A: 

Boy, this is not safe! Though valid code, but not good design! Your code allows "this" reference to escape before the object is properly constructed.

Imagine that game.addManager() would invoke some method xxx() on "this" reference. And we have a subclass of Manager, ChildManager, who overrides method xxx() and this method depends on a field in ChildManager (which isn't initialized when the super constructor reaches its last line of code). The game.addManager() would see the uninitialized value of the field in ChildManager, which is very very dangerous!

Example code:

    public class Manager {
    Game game;
    public Manager (String userName){
        game = new Game(userName);
        game.addManager(this);
    }
    public void xxx(){

    }
}

public class ChildManager extends Manager {
    String name;
    public ChildManager (String username){
        super(username);
        name = username;
    }

    public void xxx (){
        System.out.println(name);
    }
}

public class Game {
    public Game (String userName){

    }

    public void addManager (Manager m){
        m.xxx();
    }
}
Vincent