views:

269

answers:

6

Any ideas on a good way to refactor this so that my code acts the same, but without the whole throwing and catching my own exception?

public Int32 ChooseNextColor(Int32 numColors)
{
    int? nextColor = null;

    while (nextColor == null)
    {
        Console.Write("Please enter your next color selection: ");
        string input = Console.ReadLine();

        try
        {
            nextColor = Convert.ToInt32(input);
            if (nextColor > numColors || nextColor < 0) 
                throw new ArgumentOutOfRangeException();
        }
        catch
        {
            nextColor = null;
            Console.WriteLine("Unrecognized input: " + input);
            Console.WriteLine("Please enter a value between 0 and " + numColors + ".");
        }
    }

    return (nextColor.Value);
}

EDIT: The try/parse method is exactly what I am looking for.

In response to John's title edit -> I should have posted more information to begin with, and that would have been "getting rid of the try/catch all together is best". So with that in mind, I changed the title.

+1  A: 

You can use Int32.TryParse() or

if (nextColor > numColors || nextColor < 0) 
    {
        Console.WriteLine("Unrecognized input: " + input);
        Console.WriteLine("Please enter a value between 0 and " + numColors + ".");
        return null;
    }
alemjerus
+2  A: 

.NET provides TryParse for just this reason.

Anon.
Doesn't get rid of the throwing out of range when its < 0?
Sander Rijken
He is trying to get around throwing his own exception and catching it, not just the exception thrown by Convert.ToInt32
tster
Once the exception thrown by `Parse` is gone, it's an elementary change to remove the try/catch entirely. Consider it left as an exercise to the reader, if you must.
Anon.
But that was the whole point of his question. He didn't say anything about the exception from Convert.ToInt32
tster
+14  A: 

Try

int nextColor;
input = Console.ReadLine();

while( ! Int32.TryParse( input, out nextColor ) 
       || nextColor > numColors 
       || nextColor < 0 )
{ 
    Console.WriteLine("Unrecognized input: " + input);
    Console.WriteLine("Please enter a value between 0 and " + numColors + ".");
    input = Console.ReadLine();
}
Paul Alexander
This doesn't seem to perform the range validation of the original code... am I missing something?
itowlson
this doesn't check if the input is between 0 and numColors.
tster
Doesn't get rid of the throwing out of range when its < 0?
Sander Rijken
Right...added the range validation.
Paul Alexander
I would hope the original user could figure out how to add range validation to @Paul's existing code.
sixlettervariables
The question was about how to get rid of the try/catch used for range validation, not about exceptions from `Convert.ToInt32()`
Sander Rijken
I really dont like the idea of having 2 readline statements in the same function, but this is the best answer. +1 for the succinct code, I'll be using something slightly refactored from this.
NickLarsen
+1  A: 

If you want to avoid exception, you should use int.TryParse method instead of Convert.ToInt32().

Vitaly
Doesn't get rid of the throwing out of range when its < 0?
Sander Rijken
Integer can be negative.
Vitaly
+4  A: 

warning, not tested!

public int ChooseNextColor(int numColors)
{
    while (true)
    {
        Console.Write("Please enter your next color selection: ");
        string input = Console.ReadLine();
        int color;
        if (!int.TryParse(input, out color) || color > numColors || color < 0)
        {
            Console.WriteLine("Unrecognized input: " + input);
            Console.WriteLine("Please enter a value between 0 and " + numColors + ".");
        }
        else
        {
            return color;
        }
    }
}
tster
+1  A: 
    public Int32 ChooseNextColor(Int32 numColors)
    {
        var success = false;
        while (!success)
        {
            Console.Write("Please enter your next color selection: ");
            int nextColor;
            var input = Console.ReadLine();
            success = int.TryParse(input, out nextColor);

            if (success && nextColor > 0 && nextColor < numColors) return nextColor;

            Console.WriteLine("Unrecognized input: " + input);
            Console.WriteLine("Please enter a value between 0 and " + numColors + ".");
        }
        throw new ApplicationException("The thing that should not be.");
    }
Serkan