CODEWITHSUNDEEP
c#goto
keyboardNoob
keyboardNoob

Reputation: 101

How to change goto statement to improve code?

I am writing a C# Windows Form app. I have a label with a random word, e.g. "computer". User sees covered word, in this example: "--------". User has to guess what is this word by guessing letter by letter. I created button Hint. The button is responsible for revealing one random letter. That's code I created:

private void btnHint_Click(object sender, EventArgs e)
{
    repeat:
    Random rnd = new Random(); 
    int rand = rnd.Next(corrArr.Length); 
    char letter = corrArr[rand]; 

    if (letters.Contains(letter.ToString())) //check if the random letter is already used
    {
        goto repeat; 
    }
    else
    {
        word.Text = "";

        var idx = correct.IndexOf(letter);
        wordArr[idx] = Convert.ToChar(letter);

        foreach (var item in wordArr)
        {
            word.Text += item.ToString();
        }
        count++;
        
        if (wordLen == count)
        {
            this.BackColor = Color.LimeGreen; 
            result.Text = "Win!!";
            tb.Enabled = false; 
            btnClick.Visible = false; 
        }
    }
    letters.Add(letter.ToString());
}

By goto statement I can draw a letter again, if the previous one is already used.

I heard that I shouldn't use goto statement like in the beginning of code, but program works perfectly. How can I improve code?

Upvotes: 0

Views: 192

Answers (2)

benuto
benuto

Reputation: 292

Leave the if/else statement and use Do While loop

tutorialsteacher

Random rnd = new Random(); 
char letter;
do
{
      int rand = rnd.Next(corrArr.Length); 
      letter = corrArr[rand]; 
} while (letters.Contains(letter.ToString()));

 word.Text = "";

 var idx = correct.IndexOf(letter);

 wordArr[idx] = Convert.ToChar(letter);

And of course there is more to be improved... ;)

Upvotes: 3

derpirscher
derpirscher

Reputation: 17400

Make a loop around you check and repeat it while your random letter is still contained in the already used letters.

private void btnHint_Click(object sender, EventArgs e) {
    Random rnd = new Random();
    char letter;
    while (letters.Contains((letter = corrArr[rnd.Next(corrArr.Length)]).ToString()))
      ; //the loops body is left empty intentionally

    ...
}

An even better approach would be, having a collection of available letters, and everytime, you select one randomly, remove it from the collection of available letters. This way, you won't have to do any checks or loops

var availLetters = "abcdef....";

int rand = rnd.Next(availLetters.Length);
char letter = availLetters[rand];
availLetters = availLetters.Substring(0, rand) + availLetters.Substring(rand+1);

...

Upvotes: 1

Related Questions