tags:

views:

217

answers:

8

Hi folks,

in my C#-project, I have a class which contains a List

public class MyClass
{
  public MyClass parent;
  public List<MyClass> children;
  ...
}

I want to prevent the user of the class from Adding (and Deleting) an Element to the children-List, but he shall still be able to parse its elements. I want to handle the Adding and Deleting within MyClass, providing an AddChild(MyClass item) and DeleteChild(MyClass item) to ensure that, when an item is added to the child list, the parent of that item will be set properly.

Any idea how to do this besides implementing my own IList?

Thanks in advance, Frank

+3  A: 

Make the list private, and create public methods to access its content from outside the class.

public class MyClass
{
    public MyClass Parent { get; private set; }

    private List<MyClass> _children = new List<MyClass>();
    public IEnumerable<MyClass> Children
    {
        get
        {
            // Using an iterator so that client code can't access the underlying list
            foreach(var child in _children)
            {
                yield return child;
            }
        }
    }

    public void AddChild(MyClass child)
    {
        child.Parent = this;
        _children.Add(child);
    }

    public void RemoveChild(MyClass child)
    {
        _children.Remove(child);
        child.Parent = null;
    }
}

By the way, avoid declaring public fields, because you can't control what client code does with them.

Thomas Levesque
+15  A: 

If you hand the caller the List<T>, you can't control this; you can't even subclass List<T>, since the add/remove are not virtual.

Instead, then, I would expose the data as IEnumerable<MyClass>:

private readonly List<MyClass> children = new List<MyClass>();
public void AddChild(MyClass child) {...}
public void RemoveChild(MyClass child) {...}
public IEnumerable<MyClass> Children {
    get {
        foreach(var child in children) yield return child;
    }
}

(the yield return prevents them just casting it)

The caller can still use foreach on .Children, and thanks to LINQ they can do all the other fun things too (Where, First, etc).

Marc Gravell
You can always mask those methods with the new operator and make them do nothing - but you may as well just make your own custom collection by this point :)
Adam
@Adam - if you `new` them, the caller can completely bypass your methods by just typing their variable as `List<T>`. You'd also need to re-implement all of `IList` and `IList<T>`. Not worth it.
Marc Gravell
You can also use the `.AsEnumerable()` extension method instead of the `foreach`/`yield` loop. http://msdn.microsoft.com/en-us/library/bb335435.aspx
spoulson
@spoulson - no; that is implemented as "return source" (look in reflector), so I could still cast it back to a `List<T>` and call `Add`. To demo, try: `var list = new List<string>(); ((IList)list.AsEnumerable()).Add("abc"); Console.WriteLine(list.Count);`
Marc Gravell
That way you return an independent collection that gets outdated if children are added to/removed from the original List, no?
Ozan
It's better to return the list directly as the `IEnumerable<MyClass>`, as this allows LINQ to work with it efficiently. For instance, the user can use `.Count()` or `.ElementAt()` with O(1) performance. The fact that they could mutate your list by casting it is usually not important, as they could just as easily access your private fields via Reflection if you're running in full trust. The intention of the public contract is clear: I don't expect you to mutate this enumeration's collection and I make no guarantees as to what will happen if you try.
Dan Bryant
@marc - it _could_ be, but then, the caller could do the same via reflection. Usually, this isn't a security issue, but one of clarity Wrapping the list in an IEnumerable rather than exposing it as an IEnumberable is slightly longer - it depends on the usage. Internally I'd just return the list but for an external interface you might choose to use a more robust interface.
Eamon Nerbonne
@Dan / @Eamon - indeed there are pluses and minuses of it; returning the list (cast as enumerable) will *generally* suffice too.
Marc Gravell
@Ozan - no, IEnumerables implemented via `yield return` are executed lazily via a state machine. The foreach...yield return approach will yield the same results as the underlying list's iterator would in the face of concurrent modification. Modifying the list while it's not actively being enumerated is OK; modifying it while it is (i.e. you're in a foreach loop) is to be avoided.
Eamon Nerbonne
@Eamon Nerbonne: wow, I was completely ignorant of the power of the yield statement. Will definitely use it more often. (I read http://flimflan.com/blog/ThePowerOfYieldReturn.aspx)
Ozan
@Ozan - Jon's book has this available as a free chapter (ch 6): http://www.manning.com/skeet/
Marc Gravell
Many thanks, this variant suites my needs.
Aaginor
+6  A: 

Expose the list as IEnumerable<MyClass>

UpTheCreek
+1 for the KISS!
Eamon Nerbonne
A: 

You don't need to reimplemented IList, just encapsulate a List in a class, and provide your own Add and Delete functions, that will take care of adding and setting the required properties.

You might have to create your own enumerator if you want to enumerate through the list though.

Miki Watts
+1  A: 
using System.Collections.ObjectModel;

public class MyClass
{
    private List<MyClass> children;

    public ReadOnlyCollection<MyClass> Children
    {
        get
        {
            return new ReadOnlyCollection<MyClass>(children);
        }
    }
}
simendsjo
if you add/remove items on this.children, the readOnlyCollection will gain its items, as it stores the original list internally ...
Andreas Niedermair
A: 

You can encapsulate the list in the class by making it private, and offer it as a ReadOnlyCollection:

public class MyClass {

  public MyClass Parent { get; private set; };

  private List<MyClass> _children;

  public ReadOnlyCollection<MyClass> Children {
    get { return _children.AsReadOnly(); }
  }

  public void AddChild(MyClass item) {
    item.Parent = this;
    _children.Add(item);
  }

  public void DeleteChild(MyClass item) {
    item.Parent = null;
    _children.Remove(item);
  }

}

You can make the Parent a property with a private setter, that way it can't be modified from the outside, but the AddChild and DeleteChild methods can change it.

Guffa
+10  A: 

Beside of implementing your own IList<T> you could return a ReadOnlyCollection<MyClass>.

public MyClass
{
    public MyClass Parent;

    private List<MyClass> children;
    public ReadOnlyCollection<MyClass> Children
    {
        get { return children.AsReadOnly(); }
    }
}
bassfriend
+1 for the cleanest solution - though just exposing as an IEnumerable<> is almost as good, faster, and simpler.
Eamon Nerbonne
+1 from me. I like this just for the simplicity of code. KISS principle!
Srikanth Venugopalan
I like this better than enumeration via yield return, as it preserves the efficiency of list access (with only a slight hit for the indirection). I would change the return type to `IEnumerable<MyClass>`, even if `AsReadOnly()` is used, as this gives the class implementer more flexibility to change how the Children are stored/exposed in the future. If they are later placed in a Dictionary internally, I don't have to create a wrapper list just to preserve the public interface.
Dan Bryant
A: 

Use List(T).AsReadOnly().

http://msdn.microsoft.com/en-us/library/e78dcd75.aspx

Returns a read-only IList<(Of <(T>)>) wrapper for the current collection.

Available since .NET 2.0.

code4life