views:

143

answers:

5

A part of my (C# 3.0 .NET 3.5) application requires several lists of strings to be maintained. I declare them, unsurprisingly, as List<string> and everything works, which is nice.

The strings in these Lists are actually (and always) Fund IDs. I'm wondering if it might be more intention-revealing to be more explicit, e.g.:

public class FundIdList : List<string> { }

... and this works as well. Are there any obvious drawbacks to this, either technically or philosophically?

+2  A: 

Personally I would leave it as a List<string>, or possibly create a FundId class that wraps a string and then store a List<FundId>.

The List<FundId> option would enforce type correct-ness and allow you to put some validation on FundIds.

Jackson Pope
Actually the `string` class is sealed so deriving from it wouldn't be possible.
Jeff M
@Jeff M: Good catch - thanks. I've corrected my answer.
Jackson Pope
+7  A: 

I would start by going in the other direction: wrapping the string up into a class/struct called FundId. The advantage of doing so, I think, is greater than the generic list versus specialised list.

  1. You code becomes type-safe: there is a lot less scope for you to pass a string representing something else into a method that expects a fund identifier.
  2. You can constrain the strings that are valid in the constructor to FundId, i.e. enforce a maximum length, check that the code is in the expected format, &c.
  3. You have a place to add methods/functions relating to that type. For example, if fund codes starting 'I' are internal funds you could add a property called IsInternal that formalises that.

As for FundIdList, the advantage to having such a class is similar to point 3 above for the FundId: you have a place to hook in methods/functions that operate on the list of FundIds (i.e. aggregate functions). Without such a place, you'll find that static helper methods start to crop up throughout the code or, in some static helper class.

Paul Ruane
definitely the way to go. Specialize the element type, cause this needs specialization (for a `FundId`), but the list (in your case) is a list and stays as a list. Just derive from (and specialize) it if you really need a new feature in the list itself (which can't be solved by another collection class or linq).
Oliver
+3  A: 

List<> has no virtual or protected members - such classes should almost never be subclassed. Also, although it's possible you need the full functionality of List<string>, if you do - is there much point to making such a subclass?

Subclassing has a variety of downsides. If you declare your local type to be FundIdList, then you won't be able to assign to it by e.g. using linq and .ToList since your type is more specific. I've seen people decide they need extra functionality in such lists, and then add it to the subclassed list class. This is problematic, because the List implementation ignores such extra bits and may violate your constraints - e.g. if you demand uniqueness and declare a new Add method, anyone that simply (legally) upcasts to List<string> for instance by passing the list as a parameter typed as such will use the default list Add, not your new Add. You can only add functionality, never remove it - and there are no protected or virtual members that require subclassing to exploit.

So you can't really add any functionality you couldn't with an extension method, and your types aren't fully compatible anymore which limits what you can do with your list.

I prefer declaring a struct FundId containing a string and implementing whatever guarantees concerning that string you need there, and then working with a List<FundId> rather than a List<string>.

Finally, do you really mean List<>? I see many people use List<> for things for which IEnumerable<> or plain arrays are more suitable. Exposing your internal List in an api is particularly tricky since that means any API user can add/remove/change items. Even if you copy your list first, such a return value is still misleading, since people might expect to be able to add/remove/change items. And if you're not exposing the List in an API but merely using it for internal bookkeeping, then it's not nearly as interesting to declare and use a type that adds no functionality, only documentation.

Conclusion

Only use List<> for internals, and don't subclass it if you do. If you want some explicit type-safety, wrap string in a struct (not a class, since a struct is more efficient here and has better semantics: there's no confusion between a null FundId and a null string, and object equality and hashcode work as expected with structs but need to be manually specified for classes). Finally, expose IEnumerable<> if you need to support enumeration, or if you need indexing as well use the simple ReadOnlyCollection<> wrapper around your list rather than let the API client fiddle with internal bits. If you really need a mutatable list API, ObservableCollection<> at least lets you react to changes the client makes.

Eamon Nerbonne
+1  A: 

Just leave it as a List<string>, you variable name is enough to tell others that it's storing FundIDs.

var fundIDList = new List<string>();

When do I need to inherit List<T>?

Inherit it if you have really special actions/operations to do to a fund id list.

public class FundIdList : List<string> 
{
   public void SpecialAction()
   {
      //can only do with a fund id list
      //sorry I can't give an example :(
   }
}
Danny Chen
Whatever SpecialAction is, it could be an extension method.
Eamon Nerbonne
@Eamon: Sometimes he doesn't want the method called by a `List<string>`.
Danny Chen
What do you mean - called by?
Eamon Nerbonne
A: 

Unless I was going to want someone to do everything they could to List<string>, without any intervention on the part of FundIdList I would prefer to implement IList<string> (or an interface higher up the hierarchy if I didn't care about most of that interface's members) and delegate calls to a private List<string> when appropriate.

And if I did want someone to have that degree of control, I'd probably just given them a List<string> in the first place. Presumably you have something to make sure such strings actually are "Fund IDs", which you can't guarantee any more when you publicly use inheritance.

Actually, this sounds (and often does with List<T>) like a natural case for private inheritance. Alas, C# doesn't have private inheritance, so composition is the way to go.

Jon Hanna