Do you accept interfaces as constructor parameters?

Question

Does Krzysztof's recommendation apply to constructors? If so, how do you implement it properly?

We recommend using Collection, ReadOnlyCollection, or KeyedCollection for outputs and properties and interfaces IEnumerable, ICollection, IList for inputs.

For example,

public ClassA
{
    private Collection<String> strings;
    public Collection<String> Strings { get { return strings; } }
    public ClassA(IEnumerable<String> strings)
    {
        this.strings = strings; // this does not compile
        this.strings = strings as Collection<String>; // compiles (and usually runs?)
    }
}

Answer 1

You should avoid downcasting; in your example I'd suggest that the type of your contructor parameter should match (should be the same as) the type of your member data (i.e. either Collection<String> or IEnumerable<String>).

Or there's Marc's solution, which is different: because Marc's solution means that your member data isn't only a different type, it's also a different instance (i.e. if you modify the member data, then you're modifying a copy of the original collection, not editing the original collection itself; similarly if the original collection is modified after you make the copy, your local copy/member data doesn't include this subsequent change to the original collection).

Answer 2

You shouldn't use the as version - you should either accept and store something repeatable like an IList<T>, or create a new, local collection of the data:

this.strings = new Collection<string>();
foreach(string s in strings) { this.strings.Add(s); }

Actually, I'd use List<T> myself; then you can do (although this isn't the reason to use List<T>):

this.strings = new List<string>(strings);

Answer 3

If what you store is Collection, so I prefer getting ICollection as input.

Answer 4

This probably all about not exposing too much of your implementation. Ideally if you are designing a public API you should only expose the bare minimum of your implementation. If you return a full List interface there will be loads of ways for a user of the public API to mess with the innards of your classes in ways that you might not have intended.

----edit----

Answer 5

Why not accept a parameter of type IEnumerable<String>and cast it on the retrieval?

public ClassA
{
    private IEnumberable<String> strings;

    public Collection<String> StringCollection {
        get { return (Collection<String>) strings; }
    }

    public ClassA(IEnumerable<String> strings)
    {
        this.strings = strings; 
    }
}

Answer 6

I'd agree with Paige, sort of.

I agree that you should pass IEnumerable as an input (that way you support any enumerable collection).

But what you're doing above with the StringCollection property doesn't make sense to me - you're downcasting, which won't work.

-Matt

Answer 7

I believe that downcasting should be avoided if possible. If the class stores an input as a concrete class "Collection" (which means that this class works with "Collection", then I think the it is more natural to just accept an input of type "Collection" instead of the interface counterpart.

This shift the responsibility of downcasting (if needed) back to the consumer who should know what he is doing if he does downcast it.

On the other hand, I would say that accepting input as interface is better than a concrete class as it allows the flexibility of feeding in mock objects more easily. However, this would require the internals of the class depends on the interface rather than the concrete class (thus in the example, the member variable would be of IEnumerable instead of Collection).

I agree with yapiskan that Marc's solution actually has changed the usage by creating a copy of the input and thus there is a change in responsibility:

Before: the input is shared at all time between the consumer and this class. Therefore, change to the input is "shared" between the consumer and this class

After: this class's understanding of the input is detached from the consumer after the constructor is called.