non repeating random number gen - Listbox.items.contains broken?

[bind]

Greetings!

I'm having a bit of trouble with my Non repeating random number generator, basically this function will grab a start and ending number and generate random numbers between the start and end point.

private void GetRandom(int start, int max)
{
start:
if (listBox1.Items.Count == max)
{
DialogResult dlgResult = MessageBox.Show("Error: all numbers contained in range have been used, reset list?", "Reset list?", MessageBoxButtons.YesNo, MessageBoxIcon.Question);
if (dlgResult == DialogResult.Yes)
{
listBox1.Items.Clear();
foreach (Control c in this.Controls)
if (c is TextBox)
(c as TextBox).Clear();
SaveList();
}
return;
}

Random rndNumber = new Random();
int number = rndNumber.Next(start, max + 1);

if (CheckList(listBox1, number) == false)
{
listBox1.Items.Add(number);
textBox3.Text = (number.ToString());
SaveList();
}
else
{
goto start;
}
}

If I save the contents of the listbox and close the program and reopen it, its almost as if listbox.items.contains just stops working entirely... not sure what the deal is.

here's my opening func:

String[] info;
string path = Path.GetDirectoryName(Application.ExecutablePath) + @"\list.txt";
if (File.Exists(path))
{
info = File.ReadAllLines(path);


for (int i = 0; i < info.Length; i++)
{
listBox1.Items.Add(info);
}
}
else
{
File.Open(path, FileMode.Create).Close();

}

textBox1.Text = Properties.Settings.Default.start;
textBox2.Text = Properties.Settings.Default.end;

Question is this, is there some sort of internal memory the contains field works off of? is there any way to repopulate it's index?

I just dont know how else to go about this.

The app is supposed to basically keep track of previously generated numbers, saves the contents of the generated numbers in a listbox and writes to a text file on close, when the app is reopened and you click generate, shouldn't listbox.items.contains iterate through everything in the listbox to find the previously generated number?

Problem is, it's generating previously generated numbers and throwing them in the listbox, almost as if the listbox.items.contains is failing.

Any help on this will be appreciated!

Thank you!

 

[jges]

When you populate your listbox you are pulling in text from a file; when you generate a random number it is of type "int". This means that you are comparing an int to a text string. Try converting one to the other before the comparison to see if that takes care of the problem (I don't guarantee that's the problem, but it is what I would try first).

 

[bind]

Tried your suggestion, (thanks for the advise) however, the problem remains, I'm also having a secondary issue where if there's 1 number left in the list to generate, the application will go in it's loop endlessly, never generating the last integer, not sure why this is happening either.

Anyhow, I don't suppose you have any further tips?

Thanks ahead of time.

 

[jmeckley]

one big red flag for me is the "goto" statement. There is no place for "goto" or "on error" statements in .net code.

second thing I would change is directly loading generated values into the listbox. the list box is a UI element. treat it as a dumb container for showing data to a human. the actual creation and persistence of the random list of numbers should happen somewhere else.

the count of random numbers generated/persisted would influence the details of the implementation, but for the sake of argument lets say we need 20 random numbers and we want to store these numbers for later use in a text file. I would do something like this

class RandomNumberGenerator
{
   Random random = new Random();

   public int High{get;set;}
   public int Low{get;set;}

   public IEnumerable<int> GetDistinctListOfNumbers(int limit)
   {
       var result = List<int>()

       while(result.Count < limit)
       {
          var number = random.Next(Low, High);
          if(result.Contains(number)) continue;

          result.Add(number);
       }

       return result;
   }
}

class RandomNumberRepository
{
   private readonly FileInfo file;

   class RandomNumberRepository(FileInfo file)
   {
      this.file = file;
   }
   
   public void Save(IEnumerable<int> numbers)
   {
       if(file.Exists)
       {
          file.Delete();
       }

       using(var writer = file.CreateText())
       {
          numbers
            .Select(n => n.ToString())
            .Each(writer.WriteLine);
       }
   }

   public IEnumerable<int> GetRandomNumbers()
   {
       using(var reader = file.OpenText())
       {
          while((var line = reader.ReadLine()) != null)
          {
             yield return int.Parse(line);
          }
       }
   }
}

you can then compose these objects to load the listbox for display

Jason Meckley
Programmer

faq855-7190
faq732-7259

 

[SimonSellick]

One other thing: your code

private void GetRandom(int start, int max)
        {
        start:
            if (listBox1.Items.Count == max)
            { ...

tests whether the number of items in the list equals the higher limit. If the lower limit is not 1, you probably meant the difference:

private void GetRandom(int start, int max)
        {
        start:
            if (listBox1.Items.Count == max - start)
            {...

That might explain the infinite loop.

HTH
Simon.

 

[bind]

Thanks a lot for the feedback guys, you helped me come up with a more efficient, proper way of doing this.

Cheers.

 

[Zyrenthian]

If you are generating all number from x->y inclusive. Why not create an array of all the number and instead swap indexes some arbitrary number of times. It will speed it up vs determining if you already generated the number when you have 10000 numbers and only one number left to generate