Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Too many methods very similar

Tags:

c#

I have many methods which are very similar as shown in the code below:

    public static void ReadFromKeyboard(string label, out int retVal)
    {
        try
        {
            Console.Write(label);
            retVal = int.Parse(Console.ReadLine());
        }
        catch (Exception)
        {
            Console.WriteLine("Please insert int value.");
            ReadFromKeyboard(label, out retVal);
        }
    }

    public static void ReadFromKeyboard(string label, out float retVal)
    {
        try
        {
            Console.Write(label);
            retVal = float.Parse(Console.ReadLine());
        }
        catch (Exception)
        {
            Console.WriteLine("Please insert float value.");
            ReadFromKeyboard(label, out retVal);
        }
    }

    public static void ReadFromKeyboard(string label, out double retVal)
    {
        try
        {
            Console.Write(label);
            retVal = double.Parse(Console.ReadLine());
        }
        catch (Exception)
        {
            Console.WriteLine("Please insert double value.");
            ReadFromKeyboard(label, out retVal);
        }
    }

By the other hand, I don't know which method I will call. I'll discorver it only at runtime.

Is there any way I could rewrite these many methods into a single method named something like "ReadFromKeyboard" which returns either an int, a float or a double depending on the type which is passed to it as a parameter?

Thank you!

like image 275
Alex Avatar asked Sep 06 '25 05:09

Alex


1 Answers

As other answers have shown, you can eliminate the duplicated code by a variety of techniques, all of which are horrible and you should not do them.

In particular, do not attempt to use generics to solve this "problem". Generics are for situations where the code is generic. That is why they are called generics! That is, the code operates the same on every possible type. Your example is the opposite of generic code; you have different rules for a small number of types, and the way to handle that situation is to do exactly what you have already done: implement one method per different rule.

I say "problem" in quotes because you do not actually have a problem to solve here, so stop trying to solve it. Writing half a dozen similar short methods is not a major burden on authors or maintainers.

Now, that said, your code is also not as good as it could be and you should rewrite it. The correct way to write your code is:

public static int ReadInteger(string label)
{
    while(true)
    {
        int value;
        Console.Write(label);
        string read = Console.ReadLine();
        bool success = int.TryParse(read, out value);
        if (success)
          return value;
        Console.WriteLine("Please type an integer value.");
    }
}

The problems with your original implementation are:

  • Do not use exception handling as mainline control flow. Do not catch an exception if the exception can be avoided. That's what TryParse is for.
  • Do not use recursion as unbounded looping. If you want an unbounded loop, that's what while(true) is for. Remember, C# is not tail recursive by default!
  • Do not use out parameters without need. The method logically returns an integer, so actually return an integer. Rename it so that you do not get collisions with other read methods. There is no compelling benefit to making the caller write Read<int> over ReadInteger, and many compelling benefits to avoiding the out param.
like image 57
Eric Lippert Avatar answered Sep 07 '25 21:09

Eric Lippert