Circular reference in same assembly a bad thing?

Question

Assume I have the following classes in the same assembly

public class ParentClass : IDisposable
{
  public ChildClass Child
  {
    get { return _child; }
  }
}   

public class ChildClass 
{
   public ParentClass Parent
   {
     get { return _parent; }
     set { _parent= value; }
   }

   public ChildClass (ParentClass parent)
   {
     Parent= parent;
   }

}

Correct me if I am wrong but this IS bad design. Will this lead to a memory leak or some other unforseen issues later on? Apparently the garbage collector is capable of handling such kind of circular references.

Edit

What if the two classes end up getting used like this in some other class?

ParentClass objP = new ParentClass ();
ChildClass objC =new ChildClass(objP);
objP.Child = objC;

Thoughts please ....

Answer 1

Don't worry about the garbage collector; it handles reference graphs with arbitrary topologies with ease. Worry about writing objects that lend themselves to creating bugs by making it easy to violate their invariants.

This is a questionable design not because it stresses the GC -- it does not -- but rather because it does not enforce the desired semantic invariant: that if X is the parent of Y, then Y must be the child of X.

It can be quite tricky to write classes that maintain consistent parent-child relationships. What we do on the Roslyn team is we actually build two trees. The "real" tree has only child references; no child knows its parent. We layer a "facade" tree on top of that which enforces the consistency of the parent-child relationship: when you ask a parent node for its child, it creates a facade on top of its real child and sets the parent of that facade object to be the true parent.

UPDATE: Commenter Brian asks for more details. Here's a sketch of how you might implement a "red" facade with child and parent references over a "green" tree that only contains child references. In this system it is impossible to make an inconsistent parent-child relationship, as you can see in the test code at the bottom.

(We call these "red" and "green" trees because when drawing the data structure on the whiteboard, those were the marker colours we used.)

using System;

interface IValue { string Value { get; } }
interface IParent : IValue { IChild Child { get; } }
interface IChild : IValue { IParent Parent { get; } }

abstract class HasValue : IValue
{
    private string value;
    public HasValue(string value)
    {
        this.value = value;
    }
    public string Value { get { return value; } }
}

sealed class GreenChild : HasValue
{
    public GreenChild(string value) : base(value) {}
}

sealed class GreenParent : HasValue
{
    private GreenChild child;
    public GreenChild Child { get { return child; } }
    public GreenParent(string value, GreenChild child) : base(value)
    { 
         this.child = child; 
    }

    public IParent MakeFacade() { return new RedParent(this); }

    private sealed class RedParent : IParent
    {
        private GreenParent greenParent;
        private RedChild redChild;
        public RedParent(GreenParent parent) 
        { 
            this.greenParent = parent; 
            this.redChild = new RedChild(this);
        }
        public IChild Child { get { return redChild; } }
        public string Value { get { return greenParent.Value; } }

        private sealed class RedChild : IChild
        {
            private RedParent redParent;
            public RedChild(RedParent redParent)
            {
                this.redParent = redParent;
            }
            public IParent Parent { get { return redParent; } }
            public string Value 
            { 
                get 
                { 
                    return redParent.greenParent.Child.Value; 
                } 
            }
        }
    }
}
class P
{
    public static void Main()
    {
        var greenChild1 = new GreenChild("child1");
        var greenParent1 = new GreenParent("parent1", greenChild1);
        var greenParent2 = new GreenParent("parent2", greenChild1);

        var redParent1 = greenParent1.MakeFacade();
        var redParent2 = greenParent2.MakeFacade();

        Console.WriteLine(redParent1.Value); // parent1
        Console.WriteLine(redParent1.Child.Parent.Value); // parent1 !
        Console.WriteLine(redParent2.Value); // parent2
        Console.WriteLine(redParent2.Child.Parent.Value); // parent2 !

        // See how that goes? RedParent1 and RedParent2 disagree on what the
        // parent of greenChild1 is, **but they are self-consistent**. They
        // always report that the parent of their child is themselves.
    }
}