Is it a bad practice to pass "this" as an argument?

Question

I'm currently tempted to write the following:

public class Class1() 
{
    public Class1() 
    {
        MyProperty = new Class2(this);
    }

    public Class2 MyProperty { get; private set; }
}

public class Class2() 
{
    public Class2(Class1 class1) 
    {
        ParentClass1 = class1;
    }

    public Class1 ParentClass1 { get; set; }
}

Is passing "this" as an argument a sign of a design problem? What would be a better approach?

Ettieettinger

Answer 1

No, there's no fundamental design problem with passing this. Obviously, this can be misused (creating coupling that's too tight by having a related class depend on values stored in your instance when values of its own would be called for, for example), but there's no general problem with it.

Telephonist

Why is this ok in C# and not in other languages? What if Class2(Class1) decides to use the passed argument? Class1 is at this point half-constructed, i.e. in an undefined state. – Krohn

No, Class1 is guaranteed to be fully constructed at this point. Class2 has not been, though any instance initializers have been called. Anything else done in the Class2 constructor after this step, of course, has not yet occurred. – Telephonist

I must insist, if the constructor has not fully run, the object is in an undefined state. – Krohn

Everyone's entitled to an opinion, sibidiba :) Nonetheless, the constructor for the parent class is guaranteed to have run at this point, so the developer is fully aware of what has and has not been initialized. – Telephonist

@sibidiba, the object is not in an "undefined" state. The object MAY not be fully initialized from the developers POV, but they could change that if they wanted. From a language POV, the object is fully constructed and in a perfectly acceptable and defined state. – Bonehead

@tster: I understand and agree that it will compile. But is software just syntactic language? It is a random variable with an unknown distribution and an unknown non-empty domain. Sounds like undefined to me. Consider refactoring (renaming) Class1 into MarslanderController or TransactionManager. Would it still not matter to you as a programmer? – Krohn

@sibidiba: While that last comment isn't clear to me, you're simply incorrect. Every variable visible to the class at that point has a defined value. There is nothing random about it. I'm sorry, you're entitled to hold the opinion that it's "undefined", just as you're entitled to hold the opinion that the sky is purple, but both are equally incorrect. – Telephonist

I believe sibidiba's fear is that some other object would have access to your object before your constructor has finished. What if instead of doing MyProperty = new Class2(this) you did MyId = MyRegistry.Register(this)? Or what if Class2 has a registry. The concern that sibidiba is valid (although managable). There's nothing inherently wrong with passing this as a parameter, but doing so in the constructor must be safeguarded to prevent premature reference leakage. – Nucleate

Answer 2

no it is not a problem. THats why 'this' keyword exists, to allow you to pass yourself around

Teth

While I agree with the answer, I disagree with the reason. Programming languages do (and should) allow things to be expressed that are usually bad ideas (such as goto, public fields and global variables). While the rules occasionally need to be broken and languages should allow this, the available features of a language are a poor guideline as to what's generally good practice. – Abad

While I like the answer for being funny, how can it be upvoted more than most other answers? There's no valid reason stated here. And the unconditional statement is factually incorrect because of misuse, and because you have to control what parts of the object are already setup properly and which not (basically sequence of the calls passing on this and the rest of work happening the constructor) – Ssw

because its a succint answer to a succint question 'is the use of this a sign of poor design'; in general its not (as opposed to goto, overly public things; too many friends,....) – Teth

Answer 3

It's hard to say from what you've posted. The question you should ask yourself is: why does class2 need to know about class1? What types of operations is class2 going to perform on class1 during its lifetime, and is there a better way to implement that relationship?

There are valid reasons for doing this, but it depends on the actual classes.

Apiculture

Answer 4

Generally I'd prefer to hand an interface along to the 'child' class, as this reduces coupling. If Class2 really needs access to all of Class1's services and Class2 is public in this way (not fully encapsulated and constructed by Class1), then I'd consider requiring a concrete Class1 instance in the constructor a sign of a possible design issue.

Barchan

Answer 5

Just to ease your mind a bit:

Passing this as a parameter is even done by classes within the BCL. For example, the List<T>.Enumerator type holds a reference to its parent List<T> object in order to know if the list has been modified between enumerations (and hence when to throw an InvalidOperationException).

This is pretty standard when you've got two (or more) types that actually belong together in a tightly-knit , logically related group (such as the aforementioned collection and enumerator). I've seen plenty of cases where developers bend over backwards to avoid this kind of totally reasonable coupling for no practical reason.

Phenothiazine

Answer 6

No not a problem, if there is a clear need for a relationship in your design. This pattern is used often in various applications to indicate "parent" or "owner".

I've particularly used it when traversing trees in compiler implementations or in GUI toolkits.

Scyros

Answer 7

To give an example, check out the visitor pattern:

interface IVisitor {
 Visit(SomeClass c);
 Visit(AnotherClass c);
}

interface IAcceptVisitor {
  void Accept(IVisitor v);
}

public SomeClass : IAcceptVisitor {
  void Accept(IVisitor v) {
    v.Visit(this);
  }
}

Antipole

Answer 8

I do not know the memory model in C# in detail (at all). But passing this from the constructor to another object is inherently unsafe in many languages (including Java).

If you are in a constructor, the object is not constructed yet. If the other object decides to use the passed this argument at this moment, it would reference an object in an undefined state.

In your example such undefined usage does not happen, but how would you guarantee that it won't in the future? What if somebody subclasses/modifies Class2 in a manner that it uses something from ParentClass1 in its own constructor?

Krohn

You are definitely incorrect when it comes to C#, and I believe incorrect about Java as well (although I can't find a reference at the moment). Unlike C and C++, Java and C# both have clear specifications as to how fields are created in newly constructed objects. While the values of member fields may not yet be initialized, they are certainly not undefined (in the C and C++ sense of the word). – Nonperformance

@Daniel: An unitialized object is in an undefined state. It should never happen that you use a not 100% fully completely initialized object. If a this reference escapes from a constructur, that is exactly what can happen. – Krohn

The state of the object is defined; it's just not perfect. You can say with some certainty what's true and what's not true about the object at a particular point (say, right before you pass this anywhere), and if need be, could even document what functions are safe to call before the constructor's finished. But a constructor's purpose is to initialize an object -- no more, no less. It shouldn't be doing any real work; it should just be getting the object ready to do real work. – Tufted