Getting a random number that isn't the last one?

Question

So I'm trying to do a random tip generator that, obviously, shouldn't show the same tip at least until all of them have been seen before

private string getRandomTip()
{
    List<string> Titles = new List<string> {
    "You can copy the result by clicking over it",
    "Remember to press Ctrl + Z if you messed up",
    "Check web.com to update the app"};

    Random rnd = new Random();

    int Index = rnd.Next(0, Titles.Count);
    string randomString = Titles[Index];
    Titles.RemoveAt(Index);

    return randomString;
}

but for some reason this repeats the tips twice or more in a row. I've thought that I could generate random numbers until I get one that isn't the last one but this looks like a bad optimized code.

Could you please help me?

EDIT: Okay guys thank you very much I'm trying with this now

    public List<string> Titles = new List<string> {
        "You can copy the result by clicking over it",
        "This calculator was created by Glow on 21/10/17",
        "Click on this tip to show the next one"};

    private string getRandomTip()
    {
        string randomString;

        if (Titles.Count > 0) {
            Random rnd = new Random();

            int Index = rnd.Next(0, Titles.Count);
            randomString = Titles[Index];
            Titles.RemoveAt(Index);
        }
        else
        {
            randomString = "Those were all the tips!";
        }
        return randomString;
    }

and it works perfectly. Thanks again :)

Every time the method is called the list is initialized again. Removing from this temporary list won't affect anything that happens on next call. It will be filled again with all of them. You need to store the list outside the method. — Sami Kuhmonen, Oct 21 '17 at 16:51
Suggest you [read the documentation](https://msdn.microsoft.com/en-us/library/system.random(v=vs.110).aspx). Making a new random with the default constructor (same seed) will create the same sequence of random numbers. Paragraph 3 of section 1 will send you in the right direction. — J..., Oct 21 '17 at 16:52
That's right thank you very much i've updated it and looks like it works now. Thanks :) — Glow, Oct 21 '17 at 17:04
If duplicates are not enough... Random numbers can't be unique, but it does not stop people from trying - https://www.bing.com/search?q=c%23+random+unique+numbers — Alexei Levenkov, Oct 21 '17 at 17:18

score 0 · Accepted Answer · answered Oct 21 '17 at 16:59

You have two problems here. First your list is re-initialized every time you call the GetRandomTip() method so it will always be able to output from every tip possible, and ignore the call to RemoveAt(). What you can do is to only re-fill the list when it has 0 elements. This way you're sure all the tips will be shown once before resting the list.

The second one is that you shouldn't re-initialize your Random object every time. You might want to refer to this post for more info.

Here is your code, modified. It should give better results.

private class Test
{
    readonly Random Rnd = new Random();
    private List<string> _titles = new List<string>(); // Init the list with 0 elements, it will be filled-in on the first call to `GetRandomTip`

    private string GetRandomTip()
    {
        // fill the list if it's empty
        if(_titles.Count == 0) 
        {
            _titles = new List<string>
            {
                "You can copy the result by clicking over it",
                "Remember to press Ctrl + Z if you messed up",
                "Check web.com to update the app"
            };
        }

        int index = Rnd.Next(0, _titles.Count);
        string randomString = _titles[index];
        _titles.RemoveAt(index);

        return randomString;
    }
}

That's way better thanks. I had thought of using "Those were all the tips!" but refilling the list looks better. Thank you very much — Glow, Oct 21 '17 at 17:04
I think it's better to display them over and over, in case your users missed some ;-) — Mathieu VIALES, Oct 21 '17 at 17:05
If tyour question is solved please consider accepting it by clicking the check-mark. This indicates to the wider community that you've found a solution and gives some reputation to both the answerer and yourself. There is no obligation to do this. — Mathieu VIALES, Oct 21 '17 at 17:08

score 0 · Answer 2 · answered Oct 21 '17 at 17:12

You have to initialize the Random object and the list outside your method. Otherwise the list ist every time filled up on re-entering the method.

One solution could be:

public class Tip
{
    private readonly IEnumerable<string> _originalTips = new List<string>
    {
        "You can copy the result by clicking over it.",
        "Remember to press Ctrl + Z if you messed up.",
        "Check web.com to update the app."
    };

    private IList<string> _availableTips;
    private Random _random;



    public Tip()
    {
        _availableTips = _originalTips.ToList();
        _random = new Random();
    }



    public string GetRandomTip()
    {
        if (!_availableTips.Any())
        {
            _availableTips = _originalTips.ToList();
        }

        int index = _random.Next(0, _availableTips.Count);

        string tip = _availableTips[index];

        _availableTips.RemoveAt(index);

        return tip;
    }
}

Getting a random number that isn't the last one?

2 Answers2