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?

score 39 · Accepted Answer · answered Mar 16 '10 at 16:35

39

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.

answered Mar 16 '10 at 16:35

Adam Robinson

182,639
35
285
343

2

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. – sibidiba Mar 17 '10 at 00:06
3

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. – Adam Robinson Mar 17 '10 at 02:27
2

I must insist, if the constructor has not fully run, the object is in an undefined state. – sibidiba Mar 21 '10 at 21:00
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. – Adam Robinson Mar 21 '10 at 21:05
8

@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. – tster May 01 '10 at 17:11
@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? – sibidiba May 02 '10 at 17:29
10

@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. – Adam Robinson May 02 '10 at 18:16
4

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. – corsiKa Nov 18 '11 at 20:44

score 21 · Answer 2 · answered Mar 16 '10 at 16:35

21

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

answered Mar 16 '10 at 16:35

pm100

48,078
23
82
145

5

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. – dsimcha Mar 16 '10 at 23:36
1

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](http://stackoverflow.com/a/2456233/923794), 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) – cfi Mar 04 '13 at 10:42
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,....) – pm100 Mar 04 '13 at 17:27

score 8 · Answer 3 · answered Mar 16 '10 at 16:38

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.

score 8 · Answer 4 · answered May 01 '10 at 17:06

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.

score 7 · Answer 5 · answered Aug 11 '10 at 01:10

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.

score 3 · Answer 6 · answered Mar 16 '10 at 16:37

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.

score 3 · Answer 7 · answered Mar 16 '10 at 16:38

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);
  }
}

score 3 · Answer 8 · answered Mar 16 '10 at 23:19

3

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?

answered Mar 16 '10 at 23:19

sibidiba

6,270
7
40
50

2

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). – Daniel Pryden Aug 11 '10 at 01:20
@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. – sibidiba Aug 16 '10 at 14:49
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. – cHao Dec 22 '11 at 14:53

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

8 Answers8

Linked