CODEWITHSUNDEEP
c#class-design
sooprise
sooprise

Reputation: 23187

Overloading Methods Or Not?

What I have here are two methods (killZombie) that handle cases where you have one argument (string) or more than one argument (string[]). Because they do the same thing, I made another method named "killAZombie" that is used by the other two methods. The problem I'm having is that the method "killAZombie" is named... well kind of strangely. Is this a problem that other people encounter too? What is the best way to solve this and name my "KillAZombie" method something else that distinguishes itself more clearly from "killZombie"

public void killZombie(string zombieLocation){
    killAZombie(zombieLocation);
}

public void killZombie(string[] zombieLocations){
    foreach(string zombieLocation in zombieLocations){
        killAZombie(zombieLocation);
    }
}

public void killAZombie(string zombieLocation){
    //Kills a zombie at specified location
}

Another way I can see this problem being solved is by instead of overloading "killZombie" have two different methods like this:

public void killZombie(string zombieLocation){
    //Kills a zombie at specified location
}

public void killZombies(string[] zombieLocations){
    foreach(string zombieLocation in zombieLocations){
        killZombie(zombieLocation);
    }
}

This way we only have two methods that are easier to understand, but then the method isn't overloaded. In my mind, it seems like a good thing to have overloaded methods (this just means there are fewer methods, less clutter) so I'm not sure about this solution either. I'd be interested in hearing what would be the best way to tackle this problem, thanks!

Addendum:

My method actually takes 4 arguments, so the params will be at the end. The params variable is the most important one, so putting it as the last argument to make the params work seems kind of clunky. Is my concern over putting the most important argument last, legitimate enough to split up the methods into KillZombie and KillZombies or is the params still the right way to do things?

Upvotes: 4

Views: 276

Answers (6)

Eric Lippert
Eric Lippert

Reputation: 660024

Here are some ideas.

First, the C# convention for public methods is to capitalize them: "KillZombie", not "killZombie".

You can do this with just one method if you want. Here's a method that takes one or more locations. The caller can just provide a list: KillZombies(location1, location2, location3);

private void KillOneZombie(string location) { ... }
public void KillZombies(string location, params string[] additionalLocations)
{
    KillOneZombie(location);
    if (additionalLocations == null) return;
    foreach(string additionalLocation in additionalLocations)
        KillOneZombie(additionalLocation);
}

If you do want to have two methods, consider having one take an IEnumerable<string> instead of an array; that way the caller can pass in a list, a query, an array, whatever.

Your second naming pattern is more standard: KillZombie and KillZombies.

The params variable is the most important one, so putting it as the last argument to make the params work seems kind of clunky. Is my concern over putting the most important argument last, legitimate enough to split up the methods into KillZombie and KillZombies or is the params still the right way to do things?

I would think about how you expect the method to be used. Consider for example:

Console.WriteLine("User: {0} Score: {1}", user[i].Name, scores[i]);

Here we clearly expect that the "params" will be used to support a variable number of arguments in the caller. No one ever does this:

object[] results = new object[] { user[i].Name, scores[i] };
Console.WriteLine("User: {0} Score: {1}", results);

even though that is perfectly legal. If you expect that your method is going to be used like Console.WriteLine, where a varying number of parameters is going to be passed in but the number of parameters is known at compile time, then use params.

If you expect that it is going to be used with the second pattern -- someone has an array of locations -- then do not use params; make two methods, KillZombie and KillZombies, and have one of them take an IEnumerable of strings.

Upvotes: 6

Steve B
Steve B

Reputation: 37660

what about using this :

public void killZombies(string zombieLocation, params string[] zombieLocations){
    killZombie(zombieLocation);
    if(zombieLocations != null) {
        foreach(string zombieLocation in zombieLocations){
            killZombie(zombieLocation);
        }
    }
}

You can pass either one or several zombies.

[edit] as commented, this update disallow killing no zombie

Upvotes: 0

Adam Robinson
Adam Robinson

Reputation: 185643

The latter of your two choices is probably preferable in this case (given that the naming of the function implies that it's operating upon a single "zombie".

However, you might also want to look into the params keyword, just so you know what your options are. For instance, if you'd named your function simply Kill (and if it made sense to do so in this context), you could have:

public void Kill(params string[] zombieNames)
{
    foreach(string name in zombieNames)
    {

    }
}

And you could call it a number of ways:

Kill("Zoey");
Kill("Francis", "Zoey");

string[] survivors = { "Zoey", "Francis", "Bill", "Louis" };

Kill(names);

(Assuming, of course, that your survivors had all been turned into zombies!)

Also, stylistically C# code generally uses pascal casing for function names (KillAZombie rather than killAZombie).

Edit for Addendum

Yes, parameter ordering--while it has no technical relevance--is an important consideration in API design, so if you're going to be taking "less important" parameters, then you'll probably have to do without params.

With that said, I'll stand by my original recommendation: as the function is named (KillZombie versus Kill), I would stick with two versions just for the sake of making your name consistent with the parameters. I would also suggest allowing the user to specify IEnumerable<string> instead of an array. That will allow the developer to pass the names using anything that implements IEnumerable<string>, including a string array.

Upvotes: 4

LukeH
LukeH

Reputation: 269358

In this case I'd probably go with your second suggestion. KillZombie kills a single zombie; KillZombies kills multiple zombies.

Another option would be to use a single method with a params argument:

KillZombies("foo");           // kill a single zombie
KillZombies("foo", "bar");    // kill multiple zombies

// ...

public void KillZombies(params string[] zombieLocations)
{
    foreach (string zombieLocation in zombieLocations)
    {
        // kills a zombie at specified location
    }
}

(Note also that standard C# naming convention would be to use KillZombie/KillZombies with an uppercase "K".)

Upvotes: 3

Konrad Rudolph
Konrad Rudolph

Reputation: 545588

First of all, there are more than just those two alternatives.

In particular, you can use the first method without the extra method.

public void KillZombie(string zombieLocation){
    // Implement zombie killing logic here.
}

public void KillZombie(string[] zombieLocations){
    foreach(string zombieLocation in zombieLocations)
        KillZombie(zombieLocation);
}

But in this case I would recommend having two different methods. Granted, they do similar things but one takes zombies and one takes a single zombie. The method name should reflect this.

Analogously, the .NET List class has similar methods Add and AddRange.

Upvotes: 2

TomTom
TomTom

Reputation: 62093

Your example is sadly wrong - you could also use a params array to allow calls like KillZombies(location1, location2, location3). Params arrays allow an underterminedn umber of aprameters.

That said, it often is done for easier use. If you have 3 ovterloads beause they are all used, then there is nothing wrong with having them, or?

Look at the differentString.Format methods.

Upvotes: 0

Related Questions