Ensuring user input is an integer in a range I have isolated a little bit of code that was causing a small debate between myself and another user.  I have taken some of the things that he said and meshed it with the code that was being reviewed in the first place here.

Original Code


while (!validInput)
{
    var playerChoice = -1;
    Console.WriteLine("Please choose your Gesture ");
    gameSetup(listOfGestures);
    try
    {
        playerChoice = Convert.ToInt32 (Console.ReadLine());
    }
    catch (FormatException e)
    {
        validInput = false;
    }

    if (playerChoice &gt; 0 &amp;&amp; playerChoice &lt;= listOfGestures.Count)
    {
        playerGesture = listOfGestures[playerChoice - 1];
        validInput = true;
    }
    else
    {
        validInput = false;
    }
}





Here is what I came up with based on our conversation.

New Code

var validInput = false; //forgot this part
while (!validInput)
{
    var playerChoice = -1;
    Console.WriteLine("Please choose your Gesture ");
    gameSetup(listOfGestures);
    validInput = int.TryParse(Console.ReadLine(),out playerChoice);
    if (playerChoice &gt; 0 &amp;&amp; playerChoice &lt;= listOfGestures.Count)
    {
        playerGesture = listOfGestures[playerChoice - 1];
        validInput = true;
    }
}




Do I need to create a new method to sort out user input and whether or not it is an integer inside the min and max bounds?
The second sample is definitely better. I would be inclined to continue to improve the code by separating mechanisms from policies. The policy is the code that actually expresses the meaning of the program; the mechanism is the code that expresses what specific operations implement the policy.  

This idea comes from security design; you don't want the code that computes "is Bob allowed to open this door?" and the code that computes "is that really Bob's card key?" to be the same code. The first is a policy, the second is a mechanism. But it applies generally to all code.

By separating mechanism from policy we enable both to be understood more easily. The "main line" of your code should read like the specification. If you had to describe your code in English you'd probably say something like "present the user with a choice of gestures. If the user chooses an invalid gesture, keep trying until they choose a valid gesture."  That's the meaning of your program, but that's not what your code looks like. Rather, your code looks like the most important things in the world are integer and bool variables, list counts, and so on.

Let's identify a mechanism: parsing an integer and testing whether it is in range is the mechanism behind the policy of "the user must choose a valid gesture". So let's isolate that mechanism into a purely mechanistic method, that knows nothing about your policy domain:

static class Extensions
{
    public static int? BoundedParse(this string str, int lower, int upper)
    {
        if (str == null) 
            return null;
        int result;
        bool success;
        success = int.TryParse(str, out result);
        if (!success) 
            return null;
        if (result &lt; lower) 
            return null;
        if (result &gt; upper) 
            return null;
        return result;
    }
}


Let's now rewrite your loop as a do-while instead of a while. Look how much ugly mechanism we removed, leaving your policy shining brightly through:

gameSetup(listOfGestures);
int? choice;
do
{
    Console.WriteLine("Please choose your Gesture ");
    choice = Console.ReadLine().BoundedParse(1, listofGestures.Count);
}
while(choice == null);
playerGesture = listOfGestures[choice.Value - 1];


And as a bonus, we now have a reusable method that we can apply to any task that requires that a string be parsed as a bounded integer.

We can take this even further. Isn't that loop a mechanism? Move it into a helper method:

static int PromptForNumber(string prompt, int lower, int upper)
{
    int? choice;
    do
    {
        Console.WriteLine(prompt);
        choice = Console.ReadLine().BoundedParse(lower, upper);
    }
    while(choice == null);
    return choice.Value;
}


And now your method is:

gameSetup(listOfGestures);
int choice = PromptForNumber("Please choose your gesture", 1, listofGestures.Count);
playerGesture = listOfGestures[choice - 1];


Now the code is super clear what is going on at the policy level because all the stuff you don't care about is now the responsibility of some mechanism somewhere. (And again we have a great mechanism for prompting for a number that we can re-use later.)

Even better: maybe there is a better way to display a prompt and get a number. If we decided that we wanted to write a GUI instead, we can change the mechanism code without changing the policy code. The policy code doesn't care how the valid gesture gets chosen, just that it does.