If you aren't supposed to return collections to callers, how should you return a collection of data to a caller?

Question

I am writing a method that's intended to return a dictionary filled with configuration keys and values. The method that's building up this dictionary is doing so dynamically, so I need to return this set of keys and values as a collection (probably IDictionary<string, string>). In my various readings (sources escape me at the moment), the general consensus on returning collection types from method calls is not to.

I understand the reasons for this policy, and I tend to agree, but in cases like this I see no other alternative. This is my question: is there a way I can return this data to the caller, while following this principle?

Edit: The reasons I've heard for not allowing this behavior is that a collection or dictionary type that is meant to be consumed (but not modified) by the client exposes too much behavior, giving the illusion that the caller can modify the type. Dictionary for example has Add and Remove methods, as well as a mutable indexer. If the values in the dictionary are meant to be read-only, these methods are superfluous at best. Further damage can be done if the internal collection is exposed, and the 'owner' of the collection is not anticipating changes to the collection from outside sources.

There are other reasons I've heard, but I can't recall them off-hand - these are the most pertinent in my situation.

Edit: More clarification: The problem I'm having is that I'm building an API, so I have no control over the client calling this function. Cloning the dictionary isn't a problem, but I'm trying to keep my API as clean as possible. Returning a dictionary with methods such as Add and Remove implies that the collection can or should be modified, which isn't the case. Modifications here are meaningless, and so I don't want to expose the promise of that functionality through the returned type's interface.

---

Resolution: To come to terms with my desire for a clean API, I'm going to write a custom Dictionary class that does not expose the mutating methods Add and Remove, or the set indexer. This type will not implement IDictionary, but I will write a method ToDictionary that will return the data within an IDictionary. It will implement IEnumerable<KeyValuePair<TKey, TValue>> in order to have access to the standard LINQ operations over enumerables. Now all I need is a name for my custom dictionary type... =) Thanks everyone.

Answer 1

The general consensus on returning collection types from method calls is not to.

First time I've heard this, and it seems a stupid restriction to me.

I understand the reasons for this policy

Which are they then?

Edit: The reasons you cite against returning collections are specific potential problems, which can be adresses specifically (by returning a read-only wrapper), without a blanket restriction on returning collections. But as I understand your situation, the collection is actually built by the method - in that case, changes made by the caller will not affect anything else and thus aren't something you really have to worry about, nor should you be overly restrictive in what the caller is supposed to be able to do with the object created specifically for him.

Answer 2

I've never heard that advice. There might be issues with thread safety if you do it poorly, but you can work around that if you need to.

Answer 3

Check out ReadOnlyCollection() for this. Change your return type and your last statement to

return new ReadOnlyCollection(whateverYouWereReturningBefore);

Answer 4

I usually return an array of data vs a collection type. In C#, for example, a lot of the collections implement a .toArray() method, and for those that don't, an array can be retrieved using lambdas.

Edit

Saw your comment to "No Refunds No Returns" answer. If you're returning a dictionary, an array may not work for you. In this case, I would recommend returning an interface rather than a concrete implementation.

In C# (for example):

public IDictionary<string, object> MyMethod()
{
    Dictionary<string, object> myDictionary = new Dictionary<string, object>();

    // do stuff here

    return myDictionary;
}

Edit 2

You may need to implement your own read-only dictionary class and throw an exception in the necessary methods to prevent adding, etc.

In C# (Again, for example) (Not a complete solution):

public class ReadOnlyDictionary<TKey, TValue> : IDictionary<TKey, TValue>
{
    private IDictionary<TKey, TValue> _innerDictionary;

    public ReadOnlyDictionary(IDictionary<TKey, TValue> innerDictionary)
    {
        this._innerDictionary = innerDictionary;
    }

    public void Add(TKey key, TValue value)
    {
        throw new NotImplementedException();
    }

    public bool Remove(TKey key)
    {
        throw new NotImplementedException();
    }

    public void Add(KeyValuePair<TKey, TValue> item)
    {
        throw new NotImplementedException();
    }

    public void Clear()
    {
        throw new NotImplementedException();
    }

    public bool IsReadOnly
    {
        get { return true; }
    }

    public bool Remove(KeyValuePair<TKey, TValue> item)
    {
        throw new NotImplementedException();
    }

    public IEnumerator<KeyValuePair<TKey, TValue>> GetEnumerator()
    {
        return _innerDictionary.GetEnumerator();
    }

    System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator()
    {
        return _innerDictionary.GetEnumerator();
    }
}

Answer 5

In my opinion returning collections is only a problem if changing the returned collection can have side effects, eg. several functions work with the same collection.

If you are only creating the collection and not making data from a class public through returning the collection I think it is okay to simply return the dictionary

If the collection is used elsewhere in your code and the code you returned the collection to should not be able to change the collection you have to clone it.

Answer 6

The main reason for this restriction is that it breaks polymorphism, constness and access control, if the class returns a member collection. If you are building up a collection to return, and the class does not retain it as a member, then this is not a problem.

That said, you may wish to think harder about why you wish to return this collection. What do you want the calling class to be able to do with the data? Can you implement this functionality by adding methods to your class, instead of returning a collection (e.g. myobj.getvalueFromKey(s) instead of myobj.getdictionary()[s])? Might it be more appropriate to return an object that only exposes the information you want it to, rather than simply return the collection (e.g. MyLookupTable MyClass::getLookupTable() rather than IDictionary MyClass::getLookupTable()).

If you have no control over the caller, and you must return a collection of a given type, then it should either be a copy of a member collection, or a new collection entirely, that the callee doesn't store.

Answer 7

Perhaps the confusion is with readonly collections (i.e. non-mutable collections)? If so, there's an excellent series of posts by Eric Lippert that goes into good detail on how to build these.

To quote: It is much easier to reason about a data structure if you know that it will never change. Since they cannot be modified, they are automatically threadsafe. Since they cannot be modified, you can maintain a stack of past “snapshots” of the structure, and suddenly undo-redo implementations become trivial.

Answer 8

Hows about returning an IEnumerable<T>, the caller can then easily filter the results anyway they like via linq without mutating the original structure.

obviously for a Dictionary this will be IEnumerable<KeyValuePair<T,U>>

Edit: For lookup you presumebly want ToLookup() extension and ILookup

Answer 9

The reasons I've heard for not allowing this behavior is that a collection or dictionary type that is meant to be consumed (but not modified) by the client exposes too much behavior, giving the illusion that the caller can modify the type.

But it's not an illusion. The caller can modify the type (well, the instance of the type). Why on earth is this a problem?

By the same logic, DataTable.Select() shouldn't return a DataRow[], since not only can the caller manipulate the membership of that array, it can change the underlying data!

And the idea of returning an immutable dictionary-like class that has a ToDictionary() method: what possible benefit accrues from doing that?

It's true that returning immutable objects makes it possible for you to implement interning without changing your API. But that's about the only advantage that I can think of.