I’m trying to be a better developer, so I try to make sure I follow best practices as much as I can. At times when I’m just creating simple methods though, every now and then I get somewhat hung up as to what do I use as types for my input parameters and return values. You see, it is something that is so basic, we create methods all the time, but most of the time, we don’t pay much attention to it since we’d just like to continue on with the task at hand. So I’m writing this to remind myself that writing methods, sometime is not as straightforward as it seems…
A colleague was code-reviewing some project files and noticed that we have a same-intentioned method (same name, similar code on what it does but somewhat different input & return values) in 3 different classes. Of course, the placement of the code itself is erroneous, but let’s attribute that to hectic development and developer’s unfamiliarity of existing classes / methods. So, this colleague would like to have only 1 of these methods, essentially combining them. This is what the method looks like:
public static Collection<Piece> GetMembers(EntityCollection<Piece> pieces)
{
Collection<Piece> result = new Collection<Piece>();
foreach (Piece piece in pieces)
{
if (piece.IsMembers)
{
result.Add(piece);
}
}
return result;
}
|
Now, the method itself is fairly simple; it accepts an EntityCollection<Piece>, (which is a collection type that we have created for our purposes) and then gets all the entities that passes the IsMembers check. In the other 2 places, the method differs as follows:
public static Collection<object> GetMembers(Piece[] pieceList)
{
Collection<object> result = new Collection<object>();
// Same Code here...
}
public static List<Piece> GetMembers(EntityCollection<Piece> pieces)
{
List<Piece> result = new List<Piece>();
// Same Code here...
}
|
As you can see, the difference is in the return values and input parameter types. The code inside is almost identical. I can also relate with the developers that made these methods; when we create methods, we usually just take everything at face value. In most cases when the method was written, the developer has a need for it and the collection that is used by that particular is an EntityCollection, thus easily enough, the method created accepts an EntityCollection as well. All too often, this is how we write methods, in general, especially simple helper methods. Similarly, it just so happens, the consuming code at the time just needed a Collection<Piece>, so the method was created returning that same type.
Of course, looking at the above examples, this means the method is only useful if the particular input & output are that same exact type, thus from a junior developer’s perspective, it may seem to be fine to just create variants of this method; especially since .NET allows methods overloading. Keep in mind that in my scenario, these methods are not even in the same class! So the task now becomes, how can we collate all these into one nice method that can be reused? Hopefully it won’t get changed much (if at all) afterwards?
If you’re at all familiar with interfaces, after seeing these methods, the solution is very, very obvious. The solution should be something like the following:
public static IList<Piece> GetMembers(IEnumerable<Piece> pieces)
{
Collection<Piece> result = new Collection<Piece>();
foreach (Piece piece in pieces)
{
if (piece.IsMembers)
{
result.Add(piece);
}
}
return result;
}
|
The input parameter is now of type IEnumerable<Piece> and the return value is of type IList<Piece>. It is very, very obvious; but how did we get here? By looking at the disparate methods in existence, we (somewhat intuitively) know that by using interfaces, we can condense the various methods into one. Careful readers would realize that it breaks consumers of the second variant, since the second variant returns a Collection<object> but returning that as a type is bad practice anyway since the object we’re processing will always be Piece.
The question then would be, how can we get to create this method without first going through the types of consumers that is going to call it? Over time, I created some generalization rules which in hindsight seem to work fairly well. The rules for types in methods are as follows:
- Input parameters types should be of the most BASIC type that is needed by the code within the method; an interface whenever possible
- Return values should be of the most ADVANCED type possible; an interface whenever possible.
The idea behind the 2 rules are fairly simple; in the example the method code only needs to go through each item in the passed in collection and as such IEnumerable<T> fits perfectly. The code would have worked with IList<T> or ICollection<T>, but since no other methods are needed from those interfaces, between the 3 candidates we’ll choose the most BASIC type. By using the most basic type, if a consumer happens to only have access to an IEnumerable<Piece> (maybe they get it from some LINQ manipulations) this method can still be called.
For the return value, the reasoning is a bit more complex: the method can just return a type of Collection<Piece> and that can be made to work with any consuming code. After all Collection<T> implements IList<T>, ICollection<T>, and IEnumerable<T> and their non-generic variants. However, if we choose to return an actual concrete type we may possibly have trouble in the future if we needed to return something else.
One recent example of changing return values that I had encountered was with the advent of WPF; WPF has great support for data binding – but to make sure 2-way data binding works entities have to implement INotifyPropertyChanged and collections have to implement INotifyCollectionChanged. So we ended up having to change most of our methods that returns a List<T> to return ObservableCollection<T>. The methods that returned List<T>, we have to change the consumer code to store the return value as IList<T> to make it work.
So that was one lesson that I took to heart, so I try to make public methods (and even private methods as much as possible) to have interfaces are turn values. I hope that answers the question as to why I chose to return an interface versus returning an actual concrete class.
The secondary explanation that is required then is why did I chose to use an IList<T>, rather than an IEnumerable<T>? With interfaces, I have the chose between IList<T>, ICollection<T>, and IEnumerable<T>. The rule I have is that return values should be of the most ADVANCED type as possible, so I chose IList<T>. Why? That’s because in the case we have here, the collection is generated and returned – it’s not being kept, it doesn’t affect any state and in essence the returned value can be used directly by the consuming code. With that in mind, having the most advanced type returned allows consumers to immediately use the return value. If the caller wanted to remove the last entry in the list, since we’re returning an IList<T>, the caller can do it using the returned value. If we had chosen to return an IEnumerable<T>, then the caller have to create its own list and then do the removal process. So the benefit is for the consuming code, since they can act on the return value better given the more advanced type being returned.
Following my generalized rules, if the Piece entity implements an interface (say IPiece), I would change the method to return IList<IPiece> and accepts IEnumerable<IPiece>. However, this usually means most of your code should operate on the IPiece interface and rarely do so on the actual concrete Piece class itself.
Every now and then when I create methods, I go through this same thought process. I hope to make this more seamless for me in the future, so I will automatically always generate methods that are easy to consume. Hopefully this can be of use to other people.