Pet Peeve #153:Overloading and param names


I have certainly been accused of being an incredible nit-picker when it comes to API designs, but the little aesthetic things do add up.  For example consider the overload of the Equals method from System.Uri:

       public override bool Equals(object comparand);

Equals on object looks like this:

public virtual bool Equals(object obj);

Although the param names are not used in binding, the can cause developer confusion.  How many people looked at Uri’s Equals() method and asked “is this the overload of Object’s equals method or a different thing”. Not to mention it just looks sloppy, and I don’t like that. 

 

Who else is with me on this on? Do you have any stories of lost developer hours I can use to justify fixing mistakes like these in the future?

Comments (13)

  1. Albert says:

    Developer productivity is lost on bugs like the bug described here:

    http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dv_vstechart/html/vcconMixedDLLLoadingProblem.asp

    I probably spent a week working around this bug last year. And it still costs me an hour a month to explain it to various teams around the company.

    Spending time on the choosing parameter name for the Equal method is not neat picking it is something beyond that. Especially considering that you can change it any time after you have fixed all the bugs that actually cause lost productivity.

    Besides, in the age of intellisense the helpstring describing the parameter is more important than the name of the parameter itself. And if you don’t have intellisense you won’t see the parameter name either.

  2. ShadowChaser says:

    Looking to back up an internal discussion? I’m with you 100% on this, it’s definately sloppy and weird. I don’t have any actual stats for you though, sorry.

    If you want to help *right now* go hassle the developers that are doing the List<T> and Collection<T> (entire System.Collections.Generic) namespace.. NONE of it seems to match the existing collection classes! 🙁

    int ArrayList.Add(object value);

    void Collection<T>.Add(T item);

    Also, what’s the diff between a Collection and List? As far as I can tell a List is a superset of a collection, just with new methods. How can a method implementation in mscorlib or System.dll slow down or use more memory with each instance? Is there any point to having two?

    Why doesn’t ReadOnlyCollectionBase implement IList (I cant data bind it!). It’s even weirder considering ReadOnlyCollection<T> implements IList. And why not a ReadOnlyList<T>?

    My Pet Peve #154 is how the MSDN documentation doesn’t use consistent styles. Take a look at the IE or Direct-X documentation – what’s with the evil embedded frames? 🙂

    Wow, I sure went on a tangent with this comment, sorry 🙂

  3. David Levine says:

    Using different names can be confusing, but so can using non-descriptive names or poorly chosen names. WRT the question of overloading methods, I think one of the questions you should address is whether the name should reflect the actual usage, or should it use the same name as the base to preserve a naming convention.

    I agree with the goal, that names should increase understanding, and reduce, not increase confusion. But in this case, how useful is it to use the argument name "obj". What does this tell us? How does this help me use this method, and equally as important to me as a developer, how does this guide me when I want to write my own overload? I think the problem is more generic and a good guideline would call out naming conventions for arguments.

    My feeling is that if changing the name substantially increases one’s understanding of the method then it should be changed. If it is just a question of preference and it does not increase one’s understanding then it should use the base class’s name.

    As a side note, I’ve seen parameter names of method arguments used as part of the binding. I didn’t agree with the design (in fact, I don’t like it at all); the implementation used parameter names as part of the binding process to invoke methods via reflection. And it was case sensitive too. yuck.

  4. It would be even more confusing if there was parameter covariancy.

  5. David Levine says:

    I agree when it is done carelessly or for frivolous reasons. I do not agree that it should never be done – all rules have exceptions. If it truly is a requirement the language itself should enforce it.

  6. Mike Dunn says:

    I’m more peeved about template/generic examples all using List<T> instead of a clearer name like List<ContainedType>. In my experience, using Blah<T> makes it harder for newbies to grok the funky syntax. Just last week I was helping out a new hire get up to speed on some of our WTL code and he knew little about C++ templates, and it took me about 3 tries to explain what the <T> was for in all the template examples he was looking at.

  7. Aaron says:

    "… but the little *aesthetic* things do add up…."

    I’m with you on it, though.

  8. Thanks Aaron… Freudian slip no doubt, or at least posting too late at night 😉

  9. Peter Cooper says:

    "object" is equivalent of "obj" … the author basically wasted the point of naming the parameter.

    I don’t think overloaded parameter names should always line up, however the real complaint is people will often throw away opportunities for other people to read their code / design better.

    The previously mentioned <T> is another example of a wasted opportunity. The solution is to have a "code nazi", a "soup nazi" if you will, that has arbitrary rights to fix stuff like this. However, that might be too much power for one man to wield !

  10. Mark H says:

    I have to agree that consistency throughout an API is extremely important. There will always be debates as to whether parameter name ‘A’ is better than ‘B,’ but ultimately, as long as everyone uses the same name, an issue should not arise. There’s an implicity "comfort zone" with an API where even if, as David Levine pointed out, some parameters have less-than-descriptive names, the developer still understands how they are being used because the conventions are universally applied.