1
\$\begingroup\$

I've already posted a first attempt on Code review. Here is a new version that takes into account the answers I've received.

Is my program now reasonable or are there still things I need to improve?

As I am learning programming I do not want to develop bad habit. Any tips on what is good to do and what is bad to do would be grateful.

public class GuessGame
    {
    public static Random rnd = new Random();
    public int Attemp {  get; set; } = 0;
    public bool Win { get; set; } = true;
    public bool EndGame { get; set; } = true;

    public void PlayGame()
    {
        // <Program outline>
        // Ask how many attemp
        // computer generate rnd#
        // While attemps are left
        // prompt for guess
        // compare num
        // correct? jump out wrong? loop until attemps runs out  
        // end while
        // do you want to play again?
        while (EndGame)
        {

            Console.WriteLine("Lets play the Low or High Game");
            Attemp = InputAttempt();
            var comNum = GenerateComNum();
            do
            {
                // Console.WriteLine(comNum);
                var userNum = InputUserNum();
                CompareNum(userNum, comNum);

            } while (Attemp > 0 && Win);
            PlayAgain();
        }  
    }
     void PlayAgain()
    {
        label:

        Console.WriteLine("Play again? press 1 for yes/ press 2 for n");
        int again = 0;
        while (!Int32.TryParse(Console.ReadLine(), out again))
        {
            Console.WriteLine("Please input whole number");
        }
        while (again !=1 && again != 2)
        {
            goto label;
        }
        if (again == 1)
        {
            Console.Clear();
        }
        else if (again == 2)
        {
            Console.WriteLine("Good Bye Thank you for playing");
            EndGame = false;
        }
    }
    int InputUserNum()
    {
        int userNum = 0;
        Console.Write("Please guess the number btw 1-100 ?\t");
        while (!Int32.TryParse(Console.ReadLine(), out userNum))
        {
            Console.WriteLine("Invalid num please input whole num");
        }
        return userNum;
    }
    int InputAttempt()
    {
        label:
        int Attemp = 0;
        Console.Write("How many Attemps would you like?\t");
        while (!Int32.TryParse(Console.ReadLine(), out Attemp))
        {
            Console.WriteLine("Invalid num please input whole num");
        }
        if (Attemp == 0)
        {
            Console.WriteLine("Do not input 0");
            goto label;
        }
        return Attemp;
    }
    int GenerateComNum()
    {
        return rnd.Next(1, 101);
    }
    void CompareNum(int userNum, int comNum)
    {
        if (userNum == comNum)
        {
            Console.WriteLine("You have guessed right number!!\n");
            Win = false;
        }
        else if (userNum > comNum)
        {
            Attemp--;
            Console.WriteLine($"Your number is too High! \t Attemp<{Attemp}>left\n");
        }
        else if (userNum < comNum)
        {    
            Attemp--;
            Console.WriteLine($"Your number is too Low! !\t Attemp<{Attemp}>left\n");
        }
    }
    }
\$\endgroup\$
6
  • \$\begingroup\$ This isn't nice. You haven't accepted any answer under your last question yet. Is this a follow-up or why are you asking another very similar question? \$\endgroup\$
    – t3chb0t
    Oct 10 '18 at 6:55
  • 1
    \$\begingroup\$ I am sorry, I am new to this site and did not know about accepting the answer. I will go do that. After reading all the comments and I programmed again, and I wanted to get checked again. \$\endgroup\$ Oct 10 '18 at 7:07
  • \$\begingroup\$ ok, it'd be very good if you summarized the changes you've made so that people can see which suggestions you have incorporated into your code. This would make reviews much easier. \$\endgroup\$
    – t3chb0t
    Oct 10 '18 at 7:09
  • \$\begingroup\$ okay,thanks I will keep that in mind. I have one question. So after I reprogram should I post it with new question? or post it on the previous question? \$\endgroup\$ Oct 10 '18 at 7:13
  • \$\begingroup\$ If you just want to post the updated code then you can post a self-answer but please keep in mind that it needs a description too. Code-only posts are off-topic. This means that you should write something about what has changed. If you however want another review then a new question would be appropriate. Changing the code in question after receiving reviews is not allowed so posting a new question was the right move ;-) \$\endgroup\$
    – t3chb0t
    Oct 10 '18 at 7:16
4
\$\begingroup\$

A couple of things:

The word is attempt. You get it right in some places but not all. Be consistent.

You've hardcoded this for a local console. Personally I like to pass the input and output streams to the class. This gives you the flexibility to use network streams as well as a local console.

Another personal preference,having a separate method for one line of code that is only called once, I thinki, is overkill

I think you have the properties, Win, and Endgame backwards. To me if the player wins it should be set to true and if the game is entering the endgame portion then it also should be set to true.

Avoid magic numbers, either make them properties that can be set or constants that won't change.

When getting a single character input, it is much simpler to use the Read method and check if the input character is in range.

Instead of setting the Endgame property in the PlayAgain method, it would make more sense to have the PlayAgain method return a boolean.

This one applies to now and in any future endeavors. If you ever find that you absolutely must use goto, you really need to closely examine your code until you can figure out how to avoid it. goto is a very bad habit to get into.

\$\endgroup\$
3
  • \$\begingroup\$ Thank you for your advice. I understand all of your comments except one. I don't understand the where you said that "I have been hard coded this for a local console. Personally, you like to pass the input and output streams to the class" . If it's not too much trouble to ask you, could you explain on this. \$\endgroup\$ Oct 10 '18 at 4:00
  • \$\begingroup\$ @user10443653 - Basically it's about maintainability and versatility. At this point you don't know how far you'll take your code. By allowing for the possibility of communicating over a network, your class will be ready for that, if you decide to go that way. \$\endgroup\$
    – user33306
    Oct 10 '18 at 4:18
  • \$\begingroup\$ Cannot agree more. Avoid goto at all costs. More thoughts on goto here: codereview.stackexchange.com/questions/95265/… \$\endgroup\$
    – Shelby115
    Oct 10 '18 at 12:57

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.