7

Is it OK to pass this as an argument to a method while constructing an object in java?

Thinking about doing this makes me feel uneasy, but I'm not sure if it's definitely wrong. Take the following hypothetical example:

public final class A {
    private final B b;
    private final List<String> words;

    public A(B b) {
        this.b = b;
        words = new ArrayList<String>();
        for(int i = 0; i < 10; i++) {
            words.add(b.getNextStringFromUser(this));
        }
    }

    public List<String> getWords() {
        return words;
    }
}

public class B {
    // returns a String chosen by the user based on the current state of A
    public String getNextStringFromUser(A a) {
        List<String> wordsSoFar = a.getWords();
        // ... omitted
    }
}

The cases I can think of where doing this might be the right thing to do is where you want to construct an object which can be immutable from the point of view of the rest of your code, but where the constructor might take a different course depending on the state specified so far (if it makes sense to talk about the state of a partly constructed object). In the example above, a user chooses a string based on the strings chosen so far, and when they're all chosen, this object should never change again.

Is this sort of thing OK/advisable? Thanks.

user1675642
  • 747
  • 2
  • 5
  • 15
  • your example would couple class' A and B together, since in essence, A.class cannot be constructed unless you have already constructed B.class. Sometimes this isn't avoidable... but personally I try to avoid it since it makes it harder to test A.class, etc. – SnakeDoc Sep 13 '13 at 19:27
  • 1
    The problem is you cannot be sure the current object is fully constructed until its constructor finishes. So, if you pass a reference to this to another method you could be passing a partially initialized object and get into very weird and hard to debug exceptions. – Edwin Dalorzo Sep 13 '13 at 19:28
  • 1
    Based solely on the information given, it sounds like getNextStringFromUser should be in class A, and not B – Cruncher Sep 13 '13 at 19:28
  • 1
    I think the situations where you would need to this are (thankfully) few and far between. Why not pass `words` to `getNextStringFromUser()` (make it `getNextStringFromUser(List words)`)? Even given this is just an example, you shouldn't **need** to pass `A` to `B` when `A` already has a reference to `B`. So, no, it's not advisable. – MirroredFate Sep 13 '13 at 19:39
  • @EdwinDalorzo Is it true that you can't be sure? The Java spec describes in detail how a class instance is initialized. The author of the class could reasonably be sure, with careful coding, that the object is fully initialized. – fred02138 Sep 13 '13 at 19:45
  • True Fred, but you will get into trouble when others use your class not knowing it could pass references to itself out that are partially initialized. It's bad practice. – ggb667 Sep 13 '13 at 20:01
  • @fred02138 True, I may have used the wrong words. I was thinking in terms of the API consuming the reference, and not in terms of the programmer of the constructor. If they are not the same person (and even if they are) it is difficult to foresee how the object reference will be used. Once it leaves the safety of the constructor it can propagate to other objects that are counting on it to be properly initialized. So, that's what I meant. – Edwin Dalorzo Sep 13 '13 at 20:34

5 Answers5

3

as an argument to a method while constructing an object in java?

I would think twice, since your object is not even constructed fully.

It might have some attributes which are still null or unitialized. and if some other class accesses those attributes which would be null, be prepared for NULL POINTER EXCEPTIONS.

Hope this helps.

JNL
  • 4,683
  • 18
  • 29
3

Leaking the "this" reference in the constructor is dangerous, especially in a multithreaded environment. You may end up with not fully constructed object visible to other threads. It may behave ok if used from single thread, though, but bugs of this kind are realy hard to find and fix. So don't be surprised if your IDE will mark it as a warning.

Jk1
  • 11,233
  • 9
  • 54
  • 64
2

The Java language is explicit about classes are constructed. See the spec here. If you are sure that the code you are calling is only accessing state that is initialized, then it is probably safe.

fred02138
  • 3,323
  • 1
  • 14
  • 17
1

In the scenario as you have presented it, it is not acceptable.

For safety reasons, you should try to avoid passing an uninitialized/partially initialized object as much as possible. In your case, you can re-declare getNextStringFromUser(A a) to be:

public String getNextStringFromUser(List<String> wordsSoFar) {
        // ... omitted
}

Furthermore, the scenarios in which you have two objects which need to referencing each other are not common. It may be useful to check out the MVC Design Pattern and the Factory Design Pattern.

This is a similar question, except only one object has a reference to the other.

Community
  • 1
  • 1
MirroredFate
  • 12,396
  • 14
  • 68
  • 100
0

The problem in your code is not passing this to another method, the problem is that you do it in the constructor, which means your class will be accessed by another object even before it finishes initializing.

SAnDAnGE
  • 438
  • 2
  • 6