CODEWITHSUNDEEP
c#.net
Sergey Metlov
Sergey Metlov

Reputation: 26341

How to avoid public access to private fields?

For example let's declare:

private readonly List<string> _strings = new List<string>();
public IEnumerable<string> Strings
{
    get
    {
        return _strings;
    }
}

And now we can do the following:

((List<string>) obj.Strings).Add("Hacked");

So we're really not hiding the List from usage but only hide it behind interface. How to hide the list in such example without copying _strings in the new collection to restrict modification of the list?

Upvotes: 24

Views: 1048

Answers (7)

p.s.w.g
p.s.w.g

Reputation: 149068

Since no one offered this as an answer yet, here's another alternative:

private readonly List<string> _strings = new List<string>();
public IEnumerable<string> Strings 
{
    get { return _strings.Select(x => x); }
}

It's effectively equivalent to the yield return method mentioned earlier, but a bit more compact.

Upvotes: 1

Charles Lambert
Charles Lambert

Reputation: 5132

you could wrap your list in the getter like so:

private readonly List<string> _strings = new List<string>();
public IEnumerable<string> Strings {
  get {
    return new ReadOnlyCollection<string>(_strings);
  }
}

That way you can freely modify your private list inside your class, and not worry about changes outside.

NOTE: the ReadOnlyCollection does not copy the list it contains (e.g. _strings)

Upvotes: 3

Dan Tao
Dan Tao

Reputation: 128417

Plenty of ways.

The most naive approach would be to always copy the private field:

return _strings.ToList();

But that's not generally necessary. It would make sense, though, if:

  1. You happen to know that calling code will generally want a List<T> anyway, so you might as well give one back (without exposing the original).
  2. It's important for the exposed object to hold the original values even if the original collection is subsequently changed.

Alternately, you could just expose a read-only wrapper:

return _strings.AsReadOnly();

This is more efficient than the first option but will expose changes made to the underlying collection.

These strategies could also be combined. If you need a copy of the original collection (so changes to the original have no effect) but also don't want the object returned to be mutable*, you could go with:

 // copy AND wrap
return _strings.ToList().AsReadOnly();

You could also use yield:

foreach (string s in _strings)
{
    yield return s;
}

This is similar in efficiency and trade-offs to the first option, but with deferred execution.

Clearly, the key thing to consider here is how your method is actually going to be used and what functionality, exactly, you mean to be providing with this property.

* Though to be honest, I'm not sure why you'd care.

Upvotes: 35

astef
astef

Reputation: 9508

I will just post you my recents thoughts about encapsulation, may be you'll find them reasonable.

As everyone knows, encapsulation is very useful thing, it helps to fight complexicity. It makes your code logically minimalistic and clear.

As you said, protected access to the member can be broken and interface user can obtain full access to the underlying object. But what's the problem with it? Your code will not become more complicated or coupled.

Actually, as far as I know, with usage of some reflection there's almost no restrictions, even with fully private members. So why don't you try to hide them too?

I think there's no need to make a copy of a collection (or any other mutable) and approaches above are wrong. Use your initial variant:

private readonly List<string> _strings = new List<string>();
public IEnumerable<string> Strings
{
    get
    {
        return _strings;
    }
}

Upvotes: 5

Tejas Sharma
Tejas Sharma

Reputation: 3440

How about using a ReadOnlyCollection. That should fix that particular problem.

private ReadOnlyCollection<string> foo;
public IEnumerable<string> Foo { get { return foo; } }

Upvotes: 7

Matthias Meid
Matthias Meid

Reputation: 12521

Apart from ReadOnlyCollection there are truly Immutable Collections for .NET available.

Using immutable collections guarantees to the consumer that collection do not change rather than just they cannot change them. As they're immutable you can just expose them:

public ImmutableList<string> Strings { get; private set; }

More details: http://blogs.msdn.com/b/bclteam/archive/2012/12/18/preview-of-immutable-collections-released-on-nuget.aspx

Upvotes: 0

spender
spender

Reputation: 120518

ReadOnlyCollection<T> : http://msdn.microsoft.com/en-us/library/ms132474.aspx

Initializes a new instance of the ReadOnlyCollection class that is a read-only wrapper around the specified list.

Upvotes: 3

Related Questions