Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

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?

like image 589
sooprise Avatar asked Dec 06 '22 23:12

sooprise


1 Answers

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.

like image 170
Eric Lippert Avatar answered Dec 27 '22 21:12

Eric Lippert