Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

Consider Train.Passengers, what type would you use for Passengers where passengers are not supposed to be added or removed by the consuming code?

I'm using .NET Framework, so this discussion would suit .NET, but it could apply to a number of modern languages/frameworks.

In the .NET Framework, the List<Person> is not supposed to be publicly exposed. There's Collection<Person> and ICollection<Person> and guidance, which I tend to agree with, is to return the closest concrete type down the inheritance tree, so that'd be Collection<Person> since it is already an ICollection<Person>.

But Collection has read/write semantics and so possibly it should be a ReadOnlyCollection<Person>, but its arguably common sense not to alter the contents of a collection that you don't have intimate knowledge about so is it necessary?

And it requires extra work internally and can be a pain with (de)serialization.

At the extreme ends I could just return Person[] (since LINQ now provides much of the benefits that previously would have been afforded by a more specified collection) or even build a strongly-typed PersonCollection or ReadOnlyPersonCollection!

What do you do?

share|improve this question

5 Answers 5

The simplest way is to use IEnumerable<Passenger>. If you exposed a List<Passenger> as your IEnumerable, you might worry about someone casting it back to a List, but personally I wouldn't. They're breaking the contract if they do that, and you're not here to keep them from shooting themselves in the foot if they want to. Your job is to clearly communicate what they're supposed to be doing with it.

If you want to provide the ability to get a count of the passengers or to retrieve a passenger by index, then you have two options:

  1. Expose public methods/properties: public int PassengerCount { get; }, public Passenger GetPassengerByIndex(int index) (and you could get rid of the IEnumerable if you wanted to)
  2. Expose a ReadOnlyCollection<Passenger> instead of IEnumerable<Passenger> which takes a List in its constructor.

Personally I think the first option is better if you're defining a public API because it gives you more wiggle room to change your implementation without impacting your interface. Also, if the consumer has to call train.Passengers.Count, that kind of breaks one of the OO principles (about calling methods on the result of an object's methods).

There's another difference between the two options if your Train class isn't immutable. If you get a reference to train.Passengers and hold onto it, and then someone gets off the train, that change isn't reflected in the reference you have. If you use the first option then you're implying that the passenger count you get is only "as of this point in time". Therefore the first option implies mutability, but the second option implies immutability (of the passenger list, at least).

share|improve this answer
    
"Your job is to clearly communicate what they're supposed to be doing with it." -- you've successfully distilled the essence of my question, so thank you for your considerate and clear thoughts. –  Luke Puplett Jan 7 '11 at 16:56
    
I don't really know C#, but how about interface ICountableEnumerable<T> extends IEnumerable<T> { public int Count { get; } } and class CustomList<T> extends List<T> implements ICountableEnumerable<T> { ... } and then storing the passengers as CustomList but exposing them as ICountableEnumerable? –  back2dos Jan 8 '11 at 13:25

Usually when passing collections that most flexible way is to pass IEnumerable<class>

If you really need to prevent others from altering your collection that wont work though since they can cast it to the original List or whatever it was, then ReadOnlyCollection seems like a good choice.

share|improve this answer
    
Thanks for your thoughts Matt. This implies a simple array, as single-dimension arrays implicitly implement IEnumerable<T> –  Luke Puplett Jan 7 '11 at 14:56
    
Array is great if you don't want them to modify it. However it still comes down to why you don't want them to modify it. Will it break the library, is it a business rule etc etc. It might be ok to have some consuming programmer to break his own implementation but he can never be allowed to violate a business rule. –  konrad Jan 7 '11 at 14:59
    
Often enough there are some bookeeping activities that the class needs to perform. For example, if the containing class needs to register listeners for events the only way it can make sure those listeners are registered is by preventing direct access to the collection outside the class. Not to mention exposing a modifiable collection violates good OOP encapsulation. –  Berin Loritsch Jan 7 '11 at 15:37

I've never seen any Microsoft documentation that specifies that a List should not be publically exposed. If you look at the List Class definition on MSDN, you won't find any mention of this. You'll need to provide documentation to back up your assertion.

In our applications we ususally List(Of T) for our collections. They're simple, type safe and when we expose them through a web service (and such) they're automatically converted to a simple Array(Of T). We've used this approach on countless systems without much issue.

EDIT

I've read the posts in the comments, thanks to everyone for providing the documentation. For us, extending our classes isn't an issue and the fact that serialization is a pain with collections puts us in the List/IList camp.

While I agree that the recommendations generally make sense, they are just recommendations and that if your situation doesn't fit that model then you should do what works for you. In our case we expose List(Of T) or sometimes implement our own IList.

share|improve this answer
    
Hi Walter, its something that's in my head possibly from Krzysztof Cwalina's writings, and after a quick search I found this: stackoverflow.com/questions/387937/… –  Luke Puplett Jan 7 '11 at 15:23
2  
How exactly is that documentation? One person feels it's bloated vs. Microsoft's class definition... –  user7676 Jan 7 '11 at 15:28
    
It's one of the warnings that gets fired if you run Code Analysis or FxCop with the default Microsoft settings CA1002: msdn.microsoft.com/en-us/library/ms182142%28VS.80%29.aspx –  Marcie Jan 7 '11 at 15:29
    
Also see blogs.msdn.com/b/codeanalysis/archive/2006/04/27/585476.aspx for why. –  Marcie Jan 7 '11 at 15:29
    
It's an interesting twist to see the view being taken that FxCop is not documentation. –  Luke Puplett Jan 7 '11 at 15:40

In Java, you'd return a Set, and use Collections.unmodifiableSet to make the set to return. I imagine this ReadOnlyCollection/asReadOnly business is your equivalent.

IEnumerable doesn't sound like a good choice to me. I want to be able to say:

Passenger roger = rememberRoger();
if (train.getPassengers().contains(roger)) {
    tellEveryoneRogerIsOnHisWayHome();
}

That means that getPassengers has to return your equivalent of Collection.

share|improve this answer
    
I think that would work with an IEnumerable too. –  Marcie Jan 9 '11 at 17:32

I would consider the behaviours of passengers. E.g. if you don't want behaviours provided by basic types it's better to create custom types...

In Object Oriented an object should exists only if it has "behaviours" (see for example what says about this Allen Holub). So, my answer means that in my application I will design Passengers as a collection of Passenger if I don't have any additional behaviour. But once I found I need a behavoiur like "remove all Passengers under 10 years old", I'll create Passengers as a custom object with an internal collection showing a message "removePassengersYoungerThan(int years)".

I think each collection need a specific design. In the same application, too.

I hope this will be more clear than previous answer.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.