Avoiding Overloads?

A programmer, let’s call him Jehosaphat, wrote a method.
This method was intended to decide in trials whether Side A was right or Side B of any given case.

public WinningSide Decide(Side sideA, Side sideB)

Pretty awesome, eh?
Anyway, Jehosaphat decided that he wanted to be able to use Decide even if only one of the sides was present at the time of the trial. What he did was check in the method’s body whether sideA or sideB was null and returned a value accordingly.
He even wrote in the method’s documentation that either side could be null. What a hard working lad.

I’ve come to see this sort of behavior especially in Ex-VB6 writers. This is wrong. If you are reading this, never do this sort of thing unless both parameters come from an uncontrolled source (such as user input).
What you should do is create an overload for the method such as this one:

public WinningSide Decide(Side presentSide)

So that the person reading it would know that they can use only one parameter as well as two. Better readability. Better maintainability.
Case dismissed.

Advertisements

8 thoughts on “Avoiding Overloads?

  1. I don’t think using overloads results in better code. It all depends on the situation, in the case mentioned here you’d probably have to change the way you call the method from simple:

    Winner = Decide(Plaintiff, Defendant);

    to:

    if( Plaintiff == null )
    {
    Winner = Defendant;
    }
    else if( Defendant == null )
    {
    Winner = Plaintiff;
    }
    else
    {
    Winner = Decide(Plaintiff, Defendant);
    }

    Now tell me which is more readable, having the logic in one place, where it can be easily resused and possibly changed (should laws change and court cannot declare a winner unless both sides are present) or having the logic in every single place where you call it? Now you tell me which leads to code that’s easier to read and maintained…

  2. Jerry,

    I mentioned that if the input is unknown, don’t use this form and this is what you are doing. You are using variables with unknown content.

    I intended that you can use the following code:

    Side someSide = new Side();

    Winner = Decide(someSide);

    instead of having to do this:

    Side someSide = new Side();

    Winner = Decide(someSide, null);

  3. How about (assuming c++):

    // function prototype
    WinningSide Decide(Side sideA = NULL, Side sideB = NULL)
    // end

    "Decide()" would return NULL
    "Decide(sideA)" would return sideA
    "Decide(sideA, sideB)" would perform the logic to pick the best side

    That way you aren’t cluttered w/duplicate functionality and extra function definitions. No?

  4. Maybe it’s me, but I can’t see the circumstances in which a user of your method would want to call Decide(sideA).

    But it does make sense to me to allow one or both arguments to be null: the caller of your method may have object references and not know or care if they are null.

    Either I disagree or I don’t understand what you’re getting at here.

  5. I agree with Me, this is just a very bad example on when to use overloads. Come up with a better one if you think overloads result in code that’s easier to read and maintain. And no, default values are not the case either, using a Win32 example, CreateFile("test.txt") is not easier to read than CreateFile("test.txt", GENERIC_READ | GENERIC_WRITE, FILE_SHARE_READ, NULL, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL); Forcing programmers spell out the options they want makes the code more readable, since you don’t have to go through the docs to look up the defaults.

    A good example on when to use overloads is String.IndexOf(), generally you should use overloads to accept different types of arguments, not to save yourself few characters typing in default values (as you did here).

  6. "using a Win32 example, CreateFile("test.txt") is not easier to read than CreateFile("test.txt", GENERIC_READ | GENERIC_WRITE, FILE_SHARE_READ, NULL, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL); "

    mmm… yes it is.

  7. Ok, but what about the use of Optional parameters in VB.Net. I use them all of the time and I find them quite useful. Any input here?

  8. All you have acheived in this example is taking the "if equals null" code outside of the Decide function and forced the developer using the Decide function to implement it each and everytime.

    This is a poor example.

    Fall back on the big four – Inheritance, Abstraction, Polymorphism & Encapsulation.

    What you want to focus on is encapsulating the logic within the function. And even if you did provide the Decide(side) function you’d still be wise to check for nulls in your Decide(side,side) function.

    You’ve only added to the code (and hence complexity) that needs to be written.

    Perhaps you need to treat this code like a collection. Implement a TeamsCollection object. For each Team (aka Side) that plays whatever game it is Add them to the TeamCollection instance. Then implement a Decide() function that returns the winning Team.

    Then if you only add one team to the collection you have a clear winner.

    By the way, what would you intend to do for a draw?

    James.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s