While loop not working as expected for randomly picking new string from list

Question

Being a complete beginner I think I've done something seriously wrong within my WPF application as the while loop I've incorporated doesn't work as planned.

List<string> alreadyUsedReagents = new List<string>(new string[] {});
List<string> alreadyUsedMetals = new List<string>(new string[] { });

List<string> reagents = new List<string>(new string[]{
        "Hexaaqua ion",
        "Dilute NaOH",
        "Excess NaOH",
        "Dilute NH₃",
        "Excess NH₃",
        "Salt",
        "Na₂CO₃",
        "HCl"});

public void RefreshReagents()
{
    alreadyUsedReagents.Clear();
}

public string CycleThroughReagents()
{
    bool keepinLoop = true;
    string chosenReagent = null;

    while (keepinLoop == true)
    {
        int r = rnd.Next(reagents.Count);
        string pickedReagent = (string)reagents[r];


        if (!alreadyUsedReagents.Contains(pickedReagent))
        {
            alreadyUsedReagents.Add(pickedReagent);
            chosenReagent = pickedReagent;
            keepinLoop = false;
        }

        if (alreadyUsedReagents.Count == 8)
        {
            RefreshReagents();
            keepinLoop = false;
        }
        else
        {
            keepinLoop = true;
        }
    }

    return chosenReagent;
}

The while loop is suppose to loop through and return a reagent that has not been already used, all the used reagents are suppose to be stored within the alreadyUsedReagents list which the while loop analyses, then this CycleThroughReagents method is used within my SetButtonContent method which sets the button properties.

public void SetButtonContent(string ChosenMetal, TextBlock reagentText, TextBlock transMetalText,
        Button opt1, Button opt2, Button opt3, Button opt4, Button opt5, Button opt6, Button opt7, Button opt8)
{
    string pickedMetal = "Cobalt";
    string pickedReagent = CycleThroughReagents();

    reagentText.Text = pickedReagent;
    transMetalText.Text = pickedMetal;
} 

A switch function is implemented within this method which sets the tag of a specific button to 'correct' if the button matches the reagent set.

public partial class MainWindow : Window
{
    GameControl _GameControl = new GameControl();
    public string chosenMetal;
    int amtLeft = 8;


    public MainWindow()
    {
        InitializeComponent();
        chosenMetal = _GameControl.CycleThroughMetals();
        _GameControl.SetButtonContent(chosenMetal,ReagentAdded, transMetal, Opt1, Opt2, Opt3, Opt4, Opt5, Opt6, Opt7, Opt8);

        Opt1.Click += HandleButtonClicks;
        Opt2.Click += HandleButtonClicks;
        Opt3.Click += HandleButtonClicks;
        Opt4.Click += HandleButtonClicks;
        Opt5.Click += HandleButtonClicks;
        Opt6.Click += HandleButtonClicks;
        Opt7.Click += HandleButtonClicks;
        Opt8.Click += HandleButtonClicks;  
    }

    public void SwitchMetals()
    {
        chosenMetal = _GameControl.CycleThroughMetals();
        _GameControl.RefreshReagents(); //method which clears the 'alreadyUsedReagents' list
        _GameControl.SetButtonContent(chosenMetal, ReagentAdded, transMetal, Opt1, Opt2, Opt3, Opt4, Opt5, Opt6, Opt7, Opt8);
    }

    private void HandleButtonClicks(object sender, RoutedEventArgs e)
    {
        Button button = sender as Button;
        CheckForCorrect(button);

    }

    public void CheckForCorrect(Button button)
    {
        if ((string)button.Tag == "correct" && amtLeft != 0)
        {
            amtLeft -= 1;
            MessageBox.Show("You guessed correct!");
            _GameControl.SetButtonContent(chosenMetal, ReagentAdded, transMetal, Opt1, Opt2, Opt3, Opt4, Opt5, Opt6, Opt7, Opt8);
            button.IsEnabled = false;
        }

        if ((string)button.Tag != "correct")
        {
            MessageBox.Show("Oops!");
        }

        else if (amtLeft == 0)
          SwitchMetals();


    }
}

Then the above method is then used in the MainWindow to originally set the buttons and reagent text, and then the click method is used to see if the button clicked is the correct for the specific reagent that is set, and if it is then a new reagent is set. The problem I'm having is that it seems that I've completely messed up my code with the way I handle the looping and setting of the reagents, as when the reagent is displayed on the screen after each click sometimes the TextBlock appears blank and uses a previously used reagent. Any help would be seriously appreciated and I'm sorry in advance for including so much code sample but I'm seriously stumped on what to do from here.

Answer 1

Just looking at the while loop, it seems that this piece:

if (!alreadyUsedReagents.Contains(pickedReagent))
{
    alreadyUsedReagents.Add(pickedReagent);
    chosenReagent = pickedReagent;
    keepinLoop = false;
}

should be doing break. The way it is currently working does not prevent further iterations of the loop, as value that you set here in keepinloop can be overridden by the if/else below. So try replacing the line

keepinLoop = false;

with

break;

That should fix your loop.

Update. Actually instead of using some made up flag to control your loop, it might be a better idea to use some real condition, like the number of used reagents for example:

while (alreadyUsedReagents.Count != 8)
{
    int r = rnd.Next(reagents.Count);
    string pickedReagent = (string)reagents[r];

    if (!alreadyUsedReagents.Contains(pickedReagent))
    {
        alreadyUsedReagents.Add(pickedReagent);
        chosenReagent = pickedReagent;
        break;
    }
}
RefreshReagents();

if (chosenReagent == null)
{
    // make sure to come up with some default
    // or your method will return null in some cases
}