Parent element in child class

Question

The real world example is quite simple. There is a house and the house has walls and so on.

class Room {
    public List<Wall> Walls{get; set;}
}

class Wall {
    public List<WallObject> WallObjects{get; set;}
}

Now a developer has added the property room to the class wall a few years ago:

class Wall {
public List<WallObject> WallObjects{get; set;}
public Room Room{ get; set; }
}

It is very pleasant to have this object in the class. One has access to the parent element in many places (430). But I think it does not belong there and sometimes leads to errors, because you do not set or change it.

Are there are other approaches, except for methods handover in many cases.

Answer 1

There are not many ways to fix this easily, but usually I would opt to change the type of list used and use the proper events to register and unregister the parent.

public class Room
{
    public ObservableCollection<Wall> Walls { get; } = new ObservableCollection<Wall>();

    public Room()
    {
        Walls.CollectionChanged += Walls_CollectionChanged;
    }

    private void Walls_CollectionChanged(object sender, NotifyCollectionChangedEventArgs e)
    {
        switch (e.Action)
        {
            case NotifyCollectionChangedAction.Add:
                {
                    foreach (Wall w in e.NewItems)
                    {
                        w.Room = this;
                    }

                    break;
                }
            case NotifyCollectionChangedAction.Remove:
                {
                    foreach (Wall w in e.OldItems)
                    {
                        w.Room = null;
                    }

                    break;
                }
        }
    }
}

Answer 2

You're right, the information is redundant. It's always possible that the List<Wall> in Room could contain walls which Room property refer to a different room, which would be a bug. So either remove it from Wall or ensure that every Wall will be checked in the setter of Walls. If a wall's Room is != this you could throw an exception or change it.

So i'd modify the Room class a little bit:

public class Room
{
    private List<Wall> walls;

    public Room(): this(new List<Wall>())
    {
    }

    public Room(List<Wall> walls)
    {
        this.Walls = walls;
    }

    public List<Wall> Walls
    {
        get
        {
            return this.walls;
        }

        set
        {
            foreach (Wall wall in value)
            {
                if (wall?.Room != this)
                {
                    throw new ArgumentException("Every wall's room must be this Room instance", nameof(Walls));
                }
            }
            this.walls = value;
        }
    }
}

Since a room normally has 4 walls it should not be a big deal.