Click here to Skip to main content
15,891,864 members
Please Sign up or sign in to vote.
0.00/5 (No votes)
See more:
There should be a random number between 1000 to 9999 and the user should guess the number. I made 4 different text boxes for that. If the number is right and it's on the right place it has to become green. If the number's wrong it has to become red and if the number is right but it's in the wrong place it should get yellow. For example:
if the number is 1245 and the user enters 2143 it should be like this : yellow , yellow, green, red ...


This code works but the user can only guess the number once and the second time that the button is clicked there is another random number. I want it to let the user guess until all the text boxes become green. Also there is a lot of repetition in my code that i don't know how to fix... (all those if, else if , elses)

[edit]Code block added - OriginalGriff[/edit]

What I have tried:

C#
using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Windows.Forms;

namespace guess
{
    public partial class Form1 : Form
    {
        public Form1()
        {
            InitializeComponent();
        }

        private void button1_Click(object sender, EventArgs e)
        {
                Random random1 = new Random();
                int rand1 = random1.Next(1, 9);

                Random random2 = new Random();
                int rand2 = random2.Next(0, 9);

                Random random3 = new Random();
                int rand3 = random3.Next(0, 9);

                Random random4 = new Random();
                int rand4 = random4.Next(0, 9);

                if (textBox1.Text == rand1.ToString())
                {
                    textBox1.BackColor = Color.LightGreen;
                }
                else if (textBox1.Text == rand2.ToString() || textBox1.Text == rand3.ToString() || textBox1.Text == rand4.ToString())
                {
                    textBox1.BackColor = Color.Yellow;
                }
                else
                {
                    textBox1.BackColor = Color.Red;
                }

                if (textBox2.Text == rand2.ToString())
                {
                    textBox2.BackColor = Color.LightGreen;
                }
                else if (textBox2.Text == rand1.ToString() || textBox2.Text == rand3.ToString() || textBox2.Text == rand4.ToString())
                {
                    textBox2.BackColor = Color.Yellow;
                }
                else { textBox2.BackColor = Color.Red; }

                if (textBox3.Text == rand3.ToString())
                {
                    textBox3.BackColor = Color.LightGreen;
                }
                else if (textBox3.Text == rand1.ToString() || textBox3.Text == rand2.ToString() || textBox3.Text == rand4.ToString())
                {
                    textBox3.BackColor = Color.Yellow;
                }
                else
                {
                    textBox3.BackColor = Color.Red;
                }

                if (textBox4.Text == rand4.ToString())
                {
                    textBox4.BackColor = Color.LightGreen;
                }
                else if (textBox4.Text == rand1.ToString() || textBox4.Text == rand2.ToString() || textBox4.Text == rand3.ToString())
                {
                    textBox4.BackColor = Color.Yellow;
                }
                else
                {
                    textBox4.BackColor = Color.Red;
                }

                label1.Text = rand1.ToString() + rand2.ToString() + rand3.ToString() + rand4.ToString();

        }   
    }
}
Posted
Updated 3-Sep-17 23:15pm
v3

Start by doing yourself a lot of favours:
1) Create one Random instance outside the button click event handler and use that instead of four. When you create an instance of Random, it is initialized from the system clock - so unless you have an extremely slow computer, the four instances are almost certainly going to generate exactly the same sequence of numbers...
2) Stop using Visual Studio default names for everything - you may remember that "TextBox8" is the mobile number today, but when you have to modify it in three weeks time, will you then? Use descriptive names - "tbMobileNo" for example - and your code becomes easier to read, more self documenting, easier to maintain - and surprisingly quicker to code because Intellisense can get to to "tbMobile" in three keystrokes, where "TextBox8" takes thinking about and 8 keystrokes...
Don't convert numbers to strings and compare them - convert what should be numbers as strings in text boxes to numbers and compare them. That's easy to do:
C#
int valueFromTheUser;
if (!int.TryParse(myTextBox.Text, out valueFromTheUser))
   {
   // Report problem with his input to the user
   ...
   return;
   }
if (myRandomNumber == valueFromTheUser)
   {
   ...

3) Write a method which returns a bool: you pass it four numbers, and it checks if any of the the last three match the first one. Then call that three times, passing the converted textbox value. Suddenly, your repetition vanishes...
4) It's a bad "game" - the odds of the user getting it right at any time is 1:1000, which is just not going to happen: you can't generate a new random number each time he tries to guess it if you want your users to play it more than once...
 
Share this answer
 
Your code does not match your instructions. Re-read your instructions, break it down into its parts, then structure your code accordingly. For example:

1. Start/Restart button is clicked, the textboxes are cleared, and a random number is generated

2. The user enters 4 numbers and press the "guess" button

3. The code checks the values and sets the "color" state for each entry. If completed, go to step 5.

4. loop back to step 2

5. Notify the user that the game has ended.

6. Ask the user if they want to play again.

Now you are ready to write your code. Hint: there is more than one button required.
 
Share this answer
 
Hi
When working with a Random class, it's a good idea to have only one and to seed it with the current time.

Then what you're doing when you click the button is to reassign your random variables, which is why they change.

Ok, so get your logics out of your frontend and get your frontend with proper naming. I mean Form1, button1, really? and even on the code project. Let me encourage you to grow out of that fast :)

So let's invent a manger object for your random handling and a set for matching!
But first verify your input, to produce readable code which doesn't have odd errors.

private int GetValidInputOrThrow(TextBox textbox)
    {
        if (string.IsNullOrEmpty(textbox.Text) || textbox.Text.Length != 1)
            throw new ArgumentException("Only one number in each box, please", "textbox");
        return int.Parse(textbox.Text);
    }

private void button1_Click(object sender, EventArgs e)
{
    //TODO: Add a trycatch in case of validation problems
    int one = GetValidInputOrThrow(textBox1);
    int two = GetValidInputOrThrow(textBox2);
    int three = GetValidInputOrThrow(textBox3);
    int four = GetValidInputOrThrow(textBox4);

    MatchSet matchThis = MatchManager.GetMatchSet();

    if (one.Equals(matchThis.NumberOne))
        TextBox1.Backcolor = Color.LightGreen;

        //...


Now for the matchmanager class which essentially wraps an matchset and enables that to be reset using getnewmatchset, should you want that option as well.

public class RandomMatchManager
    {
        private Random _rnd = new Random((int)DateTime.Now.Ticks);
        private MatchSet _matchSet;

        public MatchSet GetNewMatchSet()
        {
            _matchSet = new MatchSet
            {
                NumberOne = _rnd.Next(0, 9),
                NumberTwo = _rnd.Next(0, 9),
                NumberThree = _rnd.Next(0, 9),
                NumberFour = _rnd.Next(0, 9)
            };
            return _matchSet;
        }

        public MatchSet GetMatchSet()
        {
            if (_matchSet == null)
                return GetNewMatchSet();
            return _matchSet;
        }
    }

    public class MatchSet : IEquatable<MatchSet>
    {
        public int NumberOne { get; set; }
        public int NumberTwo { get; set; }
        public int NumberThree { get; set; }
        public int NumberFour { get; set; }

        public bool Equals(MatchSet other)
        {
            return NumberOne.Equals(other.NumberOne) && NumberTwo.Equals(other.NumberTwo) && NumberThree.Equals(other.NumberThree) && NumberFour.Equals(other.NumberFour);
        }
    }

You will notice that the Random is kept and that the matchset offers the ability to compare the entire set, the individual values have the same from being value types. The Random is newed up with the current time tick value, to avoid getting the same values series as random really isn't necessarily that random :)

And so last bit unrevealed is making a lazy property on the form of the manager type

public partial class Form1 : Form
  {
      private RandomMatchManager MatchManager
      {
          get
          {
              if (_matchManager == null)
                  _matchManager = new RandomMatchManager();
              return _matchManager;
          }
      }
      private RandomMatchManager _matchManager;
 
Share this answer
 

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)



CodeProject, 20 Bay Street, 11th Floor Toronto, Ontario, Canada M5J 2N8 +1 (416) 849-8900