views:

246

answers:

7

I have a class which has two HashSet<String> collections as private members. Other classes in my code would like to be able to iterate over those HashSets and read their contents. I don't want to write a standard getter because another class could still do something like myClass.getHashSet().Clear(); Is there any other way to expose the elements of my HashSets to iteration without exposing the reference to the HashSet itself? I'd love to be able to do this in a way that is compatible with for-each loops.

+3  A: 

Expose a IEnumerable<T> property:

public IEnumerable<whatevertype> MyHashSet {
    get {
        return this.myHashSet;
    }
}

Of course, the user of this code can cast that IEnumerable<T> to a HashSet<T> and edit elements, so to be on the safe side (while hurting performance), you can do:

public IEnumerable<whatevertype> MyHashSet {
    get {
        return this.myHashSet.ToArray();
    }
}

or:

public IEnumerable<whatevertype> MyHashSet {
    get {
        foreach(var item in this.myHashSet) {
            yield return item;
        }
    }
}

A more performant method of protection, but less convenient to the caller, is to return an IEnumerator<T>:

public IEnumerator<whatevertype> GetMyHashSetEnumerator() {
    return this.myHashSet.GetEnumerator();
}
strager
Why even present the `ToArray` option? The iterator block is more performant and does not consume additional memory (other than for the setup of the compiler-generated stub class.)
Adam Robinson
I think the yield approach is cleanest, as it's lazy-executed and shouldn't use up more memory. I haven't measured it though.
Michael Stum
Interesting question: does `yield return` work faster than `ToArray()`?
Jeremy McGee
It's *a* method, not *the* method, and there are probably better methods out there.
strager
+3  A: 

Add a method/property like this to avoid exposing the actual container:

public IEnumerable EnumerateFirst()
{
     foreach( var item in hashSet )
         yield return item;
}
Morten Mertner
Better, as this stops /really/ obnoxious callers casting the IEnumerable back to a HashSet. However, you're not strongly typing the IEnumerable, which may not be such a good thing.
Jeremy McGee
+1. While others are advocating exposing the member only as an `IEnumerable<T>` (and this will, in a strict sense, accomplish what you want), that is not a safe practice. While the object itself is being exposed only as the interface, there's nothing stopping an enterprising developer from casting your object back to the `HashSet`. The practice here is safer, since you never expose the actual `HashSet` reference.
Adam Robinson
... but it's slower, sadly.
strager
@strager: Marginally at best.
Adam Robinson
I agree that it should be strongly typed (using IEnumerable<T>), but OP did not provide element type information so I kept the example simple.
Morten Mertner
@Morten: `HashSet` exists only as a generic type in .NET (`Hashtable` is the older, non-generic type), so it's probably a little clearer to use the appropriate `IEnumerable<T>`, though that's just my opinion.
Adam Robinson
@Morten: Actually the type was present in the source - just hidden by markdown :)
Jon Skeet
A: 

Make your getter expose the HashSet as IEnumerable.

private HashSet<string> _mine;

public IEnumerable<string> Yours
{
    get { return _mine; }
}

If the generic type is mutable, then that can still be modified, but no items can be added or removed from your HashSet.

Michael Meadows
Yes they can - the caller just has to cast back to `HashSet<string>`.
Jon Skeet
Ok, but then there's no guarantee of safety in any respect. Even if _mine is a private instance member, you can get at it using reflection. If the consumer chooses to violate the contract provided by the API, then they must accept the consequence of doing so. By exposing it as an `IEnumerable<T>`, you're making it obvious that the consumer should only be iterating, and not modifying your collection. Isn't that good enough?
Michael Meadows
@Michael: By common convention, not usually. There's a big difference between casting and reflection-based private member access.
Adam Robinson
I'm not arguing against the difference, just against over-engineering the solution. In most cases, hiding your concrete type behind an interface in a getter is sufficient. If I were a code reviewer and I saw the more "secure" solutions, I would be forced to ask if there were a solid case for going the extra mile. Most likely the answer would be "no."
Michael Meadows
@Michael: Everyone's certainly entitled to an opinion and their own practices, to be sure. It just appears (based upon the votes) that the community disagrees with you.
Adam Robinson
A: 

You may also provide a sequence like this:

public IEnumerable<string> GetHashSetOneValues()
{
    foreach (string value in hashSetOne)
        yield return value;
}

This method may then be called within a foreach loop:

foreach (string value in myObject.GetHashSetOneValues())
    DoSomething(value);
Jeffrey L Whitledge
+3  A: 

You can also use the Select method to create a wrapper than can't be cast back to HashSet<T>:

public IEnumerable<int> Values
{
    get { return _values.Select(value => value);
}

This avoids iterating over _values twice, as you would with .ToArray(), but keeps the implementation to a single clean line.

Bryan Watts
Great minds think alike :)
Jon Skeet
Hmm, interesting...
strager
+6  A: 

Assuming you're using .NET 3.5, one alternative to writing the yield code yourself is to call a LINQ method. For example:

public IEnumerable<string> HashSet
{
    get { return privateMember.Select(x => x); }
}

or

public IEnumerable<string> HashSet
{
    get { return privateMember.Skip(0); }
}

There are various LINQ operators which could be used like this - using Skip(0) is probably the most efficient, as after the initial "skip 0 values" loop, it's probably just the foreach/yield return loop shown in the other answers. The Select version will call the no-op projection delegate for each item yielded. The chances of this difference being significant are astronomically small, however - I suggest you go with whatever makes the code clearest to you.

Jon Skeet
+1. I'd go with `Skip` to avoid the unnecessary lambda overhead. While it's not terribly substantial, it doesn't really get you anything here.
Adam Robinson
+1; `Skip(0)` is a great idea!
strager
A: 

You could Clone the HashSet in conjunction with typing the property as `IEnumerable'

public IEnumerable<[private HashSet type]> HashClone
{
    get { return new HashSet<[private HashSet type]>( _privateHashSetVar ); }
}

While this protects against nefarious methods being called against the HashSet itself, it does not protect against someone iterating over a HashSet of reference types and doing something on those types. The obvious downside is that it will eat up more memory. However, if the HashSets are small, it shouldn't matter.

Thomas