I am doing some WinFX API reviews tonight and I ran across this pattern in a number of places:
/*1*/ public virtual void AddNamedParameter(string parameterName);
/*2*/ public virtual void AddNamedParameter(string parameterName, object parameterValue);
/*3*/ public virtual void AddNamedParameter(string parameterName, params object parameterValues);
Now I have not talked to the team yet, but my hunch tells me these guys are overdoing the params thing. I suspect they want to enable callers of the form:
p.AddNamedParameters (“foo”, 42, 89, 73);
That is totally goodness, but the thing is they can do this with just the 3rd overload. All those calls would work if you removed 1 and 2. The only downside is that for its params support the C# compiler always creates a temporary array so that means without overloads 1 and 2 we’d get an extra array allocation for the first two call examples I show above.
Now Rico might kill me for saying this, but that might not be a big deal at all.. creating an empty or one element array is VERY cheep, so cheep you don’t need to worry about it unless you expect the API to be used in a super tight loop. And the benift to the platform of having two less APIs to document, test, explain, support, etc is well worth it.
So, my recommendation to the team will be to remove overloads 1 and 2 and just go with the 3rd overload unless there is some “tight loop” scenario for these APIs I am missing. And if that is the case notice they need to specialize the entire implementation of 1 and 2. That is 1 can’t just turn around create a temporary array or all you have done is masked the problem from developer… and that would clearly not be pit of success work.
Oh and another thing – should all these really be virtual? We want to minimize the number of virtual members to just what is needed for the extensibility we have designed into the platform. This preserves our ability to innovate in the future. In most overload cases it is sufficient to just make the most complicated overload virtual.
What do you think, am I right or is the feature team going to “educate” me during the review?
Btw – I write this not to poke fun at the team bringing these APIs through, in fact I know those guys and they are doing an excellent job – building on the best parts of Longhorn (IMHO). I write this in the hopes that one or two of you will find some useful nugget of information that helps you write\design better managed code.