Guess a number between 1 and 100

This is my attempt at writing a guessing game. Where can I improve my code to make it more succinct?

Link to Revision 1 (Guess a number between 1 and 100 (revision 1))

```
//Program.CS
namespace MidPointGame
{
class Program
{
static void Main(string[] args)
{
RunTheGame game = new RunTheGame();
}
}
}

//Game.CS
namespace MidPointGame
{
class RunTheGame
{
int _number;

public RunTheGame()
{
const int lbound = 1;
const int ubound = 100;
int guessCount;
bool keepPlaying = true;
bool computerPlays = false;
string whoGuessed = "";

Console.WriteLine("Welcome to the numberline game");
Console.WriteLine("Try and guess the number in the fewest guesses possible");
Console.WriteLine("The boundary numbers are included in the range.");
Console.WriteLine("Lets begin");

while (keepPlaying)
{
GenerateNumber();

if (computerPlays)
{
whoGuessed = "The comupter";
guessCount = ComputerGuess(lbound, ubound);
}

else
{
Console.WriteLine($"\nGuess a number between {lbound} and {ubound}.");
guessCount = HumanGuess();
whoGuessed = "You";
}

Console.WriteLine($"\n{whoGuessed} guessed it in {guessCount} tries");

ContinuePlaying(ref keepPlaying,ref computerPlays);
}

Console.WriteLine("Thank you for playing\nPress any key to close the program.");
Console.ReadKey();
}

int HumanGuess()
{
int _guessCount = 0;
int difference = 999;

while (difference !=0)
{
difference = ValidateInput() - _number;

First, and probably the biggest problem:

RunTheGame game = new RunTheGame();

You are triggering logic in the constructor? Never, ever do class logic in the constructor! The constructor is for creating an object only. In this class, you shouldn't even have a constructor.

Let's look at a better structure for a guessing number game:


GuessingGame class

- Play() void method

- PlayAgain() bool method

- GetInput() int method

So, we have:

public class GuessingGame()
{
    public void Play()
    {
        var counter = 0;
        do
        {
            // take input with GetInput(), check input against number, etc.
            counter++;
        } while (counter < 6);
    }

    public bool PlayAgain()
    {
        Console.Write("Would you like to play again? (y/n): ");
        // read input with Console.ReadLine()
        // if the input is "y", return "true"; otherwise, return "false"
    }

    public int GetInput()
    {
        do  // loop at least once, and until we get valid input
        {
            Console.Write("Enter your guess: ");
            // read input with Console.ReadLine()
            if (int.TryParse(/* your input variable */, out int guess))
            {
                // optionally check range
                return guess;
            }
        } while (true);
    }
}

Now, we play our game like:

var game = new GuessingGame();  // calling the implicit ctor
do
{
    game.Play();
} while (game.PlayAgain());

Now for inspecting your code:

const int lbound = 1;
const int ubound = 100;

What are those doing as local constants? Those should probably be fields. They aren't specific to a single method, the whole class uses them.

Console.WriteLine("Thank you for playing\nPress any key to close the program.");
Console.ReadKey();

Wait a sec--why does your game player know when the program ends? What if you had two games, and the user was just exiting this game? Only Main should know when the program ends.

void GenerateNumber()
{
    _number = new Random().Next(1, 100);
}

Make a field for Random and only assign it once. Each time you use it, it reseeds itself so it will really be random. You are using the local clock to seed it here, so it will product the same value if you call it twice in a single second.

In HigherOrLower(int difference), you should just return the result directly from the switch. Don't assign a variable, then return at the end of the method.

throw new System.FormatException(); //Is there a better way of doing this?

Sure there is. Check the input and notify the user, and let them try again. And do it in a dedicated method. Maybe you can modify the stub GetInput() method I gave you into GetInput(int minValue, int maxValue) and make sure it is within the ranges provided. This is a perfect example of extracting similar logic into a more generic function.

int.Parse will throw an exception if the user types a non-number. Use int.TryParse to see if it is a valid number, and make the user input a valid number.

ValidateInput appears to be reading input too... Use a better name.

There appears to be a lot more stuff I don't have time to review. However, this should provide you plenty to chew on for now.

StackExchange Code Review Q#158247, answer score: 12