If you have ever written code that is going to be used as an API for other programmers, you may start to think about writing code in a different viewpoint from what you normally do.
Most of us write code for the purpose of achieving a goal. If we practice writing elegant code, we are conscious of making that code as readable and terse as possible.
Seldom do we think about the use of our code from an API standpoint.
There is a subtle difference between designing your code in a way that will make it easier for someone else to maintain, and designing your code in a way that will make it easier for someone else to use.
Intellisenselessness
How often are you working against some API and you type a method name you want to use only to have intellisense present you with 5 overloads for the method all with several different parameters choices?
Which one do you use? It is hard to be sure, you end up having to read through the long lists of parameters to figure out what method you should call and what parameters you should pass it.
Wouldn?t it be better if you were presented with what the method does in the method name rather than guessing what it does in the parameter list? Something like this?
Perspective
From the perspective of the person writing the Login method(s), overloads probably seem like the most efficient and correct way to implement the multiple ways the method can be called.
From the perspective of the person using the Login method(s), additional methods are much preferred, because they are easier to understand and know what you are looking for.
Try to think from the perspective of someone using your code when writing your code.
Extract boolean parameter to two methods
I want to take a look at a very specific example that can be of great benefit to the readability and use of your code.
Take a look at this code below.
1: public void Login()
2: {
3: }
4:
5: public void Login(bool rememberMe)
6: {
7: }
Seems like a fine overload of a Login method. I have written code just like this, you probably have also.
Unfortunately, by adding this overload, we have added some complexity to our API, because now the user of that code has to see that there is an additional overload that take a bool parameter called rememberMe.
Consider this longer alternative.
1: public void LoginRememberMe()
2: {
3: }
4:
5: public void LoginDoNotRememberMe()
6: {
7: }
Instead of overloading Login and making the user have to decide which overload to call and pass in a parameter, we have elected to create two differently named methods which take 0 parameters and clearly state what they are going to do.
I?m not saying you should never write overloads, but anytime you see an overload in your code base, you should stop and think if it would be more clear to make that overloaded method into two different methods that can eliminate one or more of the parameters.
Any time you are restricting the number of choices someone using your code has to make, you are making that code simpler to use.
If you don?t believe me, consider why the iPhone has only one button.
This is basically exactly what Objective-C and Cocoa does. This, along with separating the name of the method when you have multiple parameters, makes the code a lot more readable.
In most cases, I believe many overloads is an anti-pattern. It may indicate that you can model it differently, such as sending in a configuration object instead.
In your example, you’ve just moved the problem to the method level. Although better than many overloads, you now get an explosion of methods that basically do the same thing, or very similar things.
Do you really need to be able to send in a string url, or a Uri object that represents the same thing.
@Henning Anderssen
Definitely, you are correct. Often I transform many overrides to a builder. A fluent style builder is nice in many situations.
Initially I thought overloads were great (I had come from a VB 6 background in the land of optional params – which have now been introduced to c#4..hmmm) and I now actively discourage them for the reasons you have stated. Like Henning, I also believe it to be an anti-pattern. Although this isn’t mentioned specifically, I strongly recommend the book ‘Clean Code’ by Robert Martin (Uncle Bob) – it talks in details about these kind of things and it really makes you think about how you write your code.
@Dave Masters
Thanks Dave. I agree with you ‘Clean Code’ is an excellent book. I highly recommend it.
One of the key things that he talks about in that book is the idea that parameters are bad. He says to strive for 0 parameter methods. When I started writing code actively focusing on reducing the number of parameters, I realized how easy it was to use my code.
@John Sonmez
I’d even dare to say, that if you want to refactor towards more meaningful names of methods, probably you should introduce fluent interface. For your example I’d consider:
public interface IMainObjectInterface
{
// blah blah
ILogin Login();
}
public interface ILogin
{
void DoNotRememberMe();
void RememberMe
}
Nowadays, if I have any ‘method based’ doubt what should I do, I write “.” and hope that someone wrote a fluent interface 😉
This does make is messier if you have logic deciding which one to use, though.
if (rememberMe)
this.LoginRememberMe();
else
this.LoginDoNotRememberMe();
Optional parameters is just as evil as many method overloads.
Sometimes method overloads is required, but IMO you should limit yourself to 1-2 overloads at the most.
I’ve created lots of overloads myself many times, and it’s easy to fall into the trap.
The mentioned alternative methods here adds more complexity. If you do need several “options”, I would probably introduce a new class called LoginOptions, where username, password, rememberme and Url is listed as properties. That would increase readability, refactorability plus the api gets better.
public void Login(LoginOptions options)
{
//do the magic here
}
@Szymon Kulec
Now that I like! I didn’t want to go to far with it in this post, but I agree with you about the fluent interface.
@Henning Anderssen
I have done as you suggest before, but consider the user of your code in this case. If you create a login options class, they will have to instantiate it and know about that class. Sometimes this trade off is ok, but many times the user would rather just have more methods.
Consider the original xml classes in the .NET library.
Although this isn’t too bad of an alternative…
Login(LoginOptions.WithUserName(“joe”).WithPassword(“pass”).WithStartPage(“home”));
I am curious what role the new “Optional Parameters” could play in this discussion, and if they could be used to create a happy medium between method overload and the multiple methods.
Optional parameters in .NET 4.0 add whole new dimension to this. I prefer them because from a coding stand point there is a lot more reuse.
RE: iPhone, give me a break! Your point is void; it is a touch screen with virtually unlimited buttons.
@Adam dR.
Optional parameters are still not very friendly to the user of your code IMO. They still have to figure out what the optional parameters mean, better to have more method names that explicitly state what the method does than parameters which the user has to look up the meaning of.
As for the iPhone. I see your point, but… consider this. While you are right it is a touch screen with virtually unlimited buttons. The operating system itself has only one button. Even though apps can create their own buttons on the screen, the OS is only responding to the single button. (Aside from powering off and volume control.)
As for the “refactor boolean parameters to different methods”:
Another good advice is to generally prefer an enumeration to a boolean parameter, because it makes it easy to add a third option without breaking the API and without copying parts of existing methods to create a new one just for this option.
Creating a new Method for each possible combination of options quickly ends in a combinatory explosion and I’m sure we don’t want that.
@John Sonmez
I completely agree with Wanja on the use of enumerations to help simplify the API and make it more expressive.
I also want to note that I agree with you John, on the trying to make a cleaner API. One thing I am a fan of is the new push for fluent interfaces. (http://msdn.microsoft.com/en-us/magazine/ee291514.aspx) I think in some ways this follows some of the same principles you layout in your article.
I do however think that if you name your parameters correctly their intent will be obvious. Another thing I like about optional/named parameters is their ability to be expressive on the calling side. Much like object initializers are used in the article link above so can optional/named parameters.
I guess I would concede the iPhone discussion saying there are two buttons. The [] button and the screen itself. I get your point about simplicity I am just having a hard time drawing the parallel between hardware and software.
@Adam dR.
Yes Adam, good points. I also agree with Wanja about the use of enumerations.
I’m all in favor of having multiple methods as your public API — instead of one gigantic set of overloads. If you really want a gigantic overload method there’s no reason why you couldn’t do it as a private method that each of the public methods would call. Yes, it adds more code, but I feel the code is easier to understand by both the public API and the internal class. I feel that by making the overloaded method public you are broadcasting to much information about your specific implementation. The descriptive method names encapsulate your implementation — that means you are free to change the back-end it all you want! If you did an overload you _probably_ would have to continually adjust the public API as well when the back-end changes.
A guideline that I like to follow with overloads is this:
1) Overloaded methods should have exactly the same behavior.
2) Since overloaded methods have the same behavior all methods can be implemented in terms of one of the methods, usually this is the method that takes the most low level parameters.
A good example of this pattern is the System.IO.TextWriter class. All implementations of Write and WriteLine eventually boil down to uses of the Write(char) method.
I would prefer to use the TextWriter as written, rather than a class with umpteen methods like WriteInt32, WriteLong, WriteChar, WriteDouble, etc.
I was skimming through Martin Fowler’s Refactoring book and noticed this quote under ‘Replace Parameter with Explicit Methods’ on page 285.
“The usual case for [Replace Parameter with Explicit Methods] is that you have discrete values of a parameter, test for those values in a conditional, and do different things. The caller has to decide what it wants to do by setting the parameter, so you might as well provide different methods and avoid the conditional. You not only avoid the conditional behavior but also gain compile time checking. Furthermore your interface also is clearer. With the parameter, any programmer using the method needs not only to look at the methods on the class but also to determine a valid parameter value. The latter is often poorly documented.”
This isn’t directly related to overloaded methods, but I believe that overloaded methods are near equivalent to one method with several conditional parameters.
With .Net 40 and optional parameters, this is taken to the next level of complexity. I guess we’ll just have to see how they will scale and be maintained.
Wasn’t one of the reasons for optional parameters was to help with the complexity for COM operability?
John,
I don’t necessarily see that as a bad thing.
Consider the following method:
public void DoSomethingWithUser(string firstname, string middlename, string lastname, int age);
When reading that, how do you know which string is which. What if the creator of that method decides to swap the name string. You get no compiletime checks, and the signature is harder to read.
Now, take this method signature instead. It is much easier to read and you know just by reading the signature what it accepts, plus it gives you compiletime checks if you decide to move the parameter.
public void DoSomethingWithUser(Name fullName, int age);
Btw, we’ve all been under the assumption that you have to pass in the parameters into the method. In many cases, you’d want to do that, but sometimes wouldn’t it be better to have the class statefull instead.
Case in point, lets take the Login example. Do you REALLY need to send in the parameters? Can’t you just store the parameters in the class instance instead. That would mean the login method would have no parameters. Nothing beats that ;).
It won’t work all the time in all usecases, but it is something you should consider when you create your api/system.
@John Atwood
I definitely have to agree with you there John. Good example. Yes, if the overload does exactly the same thing and only differs in type, it should be an overload rather than an additional method.
@Henning Anderssen
Good point!
I agree wholeheartedly with your reasoning and example.
In fact my next post is going to be on constraining user input through the use of custom parameter classes.
I don’t think what I am talking about in my post and what you are saying here are necessarily mutually exclusive though. It kind of depends on the situation and what data your storing and how your API is interacted with. I could easily see a login method that takes in a fullName, but is an additional method instead of an overload.
I agree also that you can store parameters in the class instance, but you will still have to get them in there somehow. Which can jump move the problem up to overloaded constructors.
Good points though excellent discussion.
I agree with your refactoring; I think the code smell is the Boolean parameter, though. If your method has a Boolean parameter, it’s probably doing more than one thing (thus violating SRP). Enum parameter types are also potential code smells.
“Try to think from the perspective of someone using your code when writing your code” – There is an easy way to do this: write tests first, and you’ll see if using teh api over and over again in different contexts is making sense.