C# populate array with unique ints No Linq or ArrayLists;

Question

This code is buggy but can't figure out why ... want to populate an array with 7 unique random integers without using arraylists or linq! I know the logic is not okay...

class Program
{
    static void Main(string[] args)
    {    int current;
         int[] numbers = new int[7];  // size of that array 
         Random rNumber = new Random();
         current = rNumber.Next(1, 50);
         numbers[0] = current;
         Console.WriteLine("current number is {0}", current);
         for (int i=1;i<7;i++)
         {
             current = rNumber.Next(1, 50);
             for (int j = 0; j < numbers.Length; j++)
             {
                 do
                 {
                     if (current == numbers[j])
                     {
                         Console.WriteLine("Duplicate Found");
                         current = rNumber.Next(1, 50);
                     }
                     else
                     {   
                         numbers[j++] = current;
                         break;
                     }
                 }while (current == numbers[j]);

             }//inner for

         }//outer for
         for (int l = 0; l < 7; l++) // DISPLAY NUMBERS
         {
             Console.WriteLine(numbers[l]);
         }

    }// main
 }//class

Have you tried stepping through with a debugger?

Daniel Mann
– Daniel Mann

2011-11-11 20:09:47 +00:00
Commented Nov 11, 2011 at 20:09 — Daniel Mann
– Daniel Mann, Commented Nov 11, 2011 at 20:09

Joe · Accepted Answer · 2011-11-11 21:06:00Z

4

want to populate an array with 7 unique integers without using arraylists or linq!

int[] list = new int[7];
for (int i = 0; i < list.Length; i++)
{
    list[i] = i;
}

EDIT

I changed your inner loop, if the random number is already in the array; create a new random and reset j to 0.

        for (int i = 1; i < 7; i++)
        {
            current = rNumber.Next(1, 50);
            for (int j = 0; j < numbers.Length; j++)
            {
                if (current == numbers[j])
                {
                    Console.WriteLine("Duplicate Found");
                    current = rNumber.Next(1, 50);
                    j = 0; // reset the index iterator
                }
            }//inner for
            numbers[i] = current; // Store the unique random integer
        }//outer for

edited Nov 11, 2011 at 21:06

answered Nov 11, 2011 at 20:01

Joe

82.8k18 gold badges130 silver badges147 bronze badges

Sign up to request clarification or add additional context in comments.

6 Comments

Chris Shouts Over a year ago

We all know that the OP meant 7 unique random integers, however this is certainly the best answer to the question that was actually asked. +1

Marin Over a year ago

I meant Random gentlemen my apologies

Joe Over a year ago

Downvoters, please explain your hate. Or explain where I'm wrong. Thanks

Casperah Over a year ago

It is not generating random numbers between 1 and 50 and there is no garanties that there is no duplicates.

Joe Over a year ago

@Casperah, where is 1, 50 a requirement?

|

Tevo D · Accepted Answer · 2011-11-11 20:16:38Z

I presume you are looking for random numbers, so the other answer is not what you are looking for.

There are a couple of issues here.

The inner loop is testing for duplicates. However, it is looking from 0 through the end of the array since it is using numbers.length. This should probably be i, to compare with already set values. numbers.length is always 7 regardless of whether or not you set any of the elements.
the assignment is using j, so presuming the first element is not a duplicate, it will be overwritten each time. That should be numbers[i] = current;. No ++ necessary as the for is handling the incrementing.
if you determine that a number is a duplicate, j should be reset to zer to check against the entire list again rather than having the while in the middle.

Without a complete rewrite, the changes will look something like this:

     for (int i=1;i<7;i++)
     {
         current = rNumber.Next(1, 50);
         for (int j = 0; j < i; j++)  //----------------- loop through set values
         {
             if (current == numbers[j])
             {
                 Console.WriteLine("Duplicate Found");
                 current = rNumber.Next(1, 50);
                 j = 0; // -----------------------reset the counter to start over
             }
         }//inner for

         // if we got here there is no duplicate --------------------------------
         numbers[i] = current;

     }//outer for

(Please note that I have not tested this code, just added the changes)

Tom · Accepted Answer · 2011-11-11 20:14:37Z

you keep overwriting the same indexes in the else, and also checking too many indices causing the first to show up as a duplicate at all times which was false...

change it to:

 for (int i=1;i<7;i++)
 {
     current = rNumber.Next(1, 50);

     for (int j = 0; j < i; j++)    ///< change to j < i.  no need to check the others
     {
         do
         {
             if (current == numbers[j])
             {
                 Console.WriteLine("Duplicate Found");
                 current = rNumber.Next(1, 50);
             }
             else
             {   
                 numbers[i] = current;   ///< not j++ but i to prevent writing at the same locations over and over again
                 break;
             }
         }while (current == numbers[j]);
     }//inner for
 }//outer for

Nate · Accepted Answer · 2011-11-11 22:44:32Z

0

What about this?

int[] list = new int[7]; 
var rn = new Random(Environment.TickCount);
for (int i = 0; i < 7; i++) 
{ 
    var next = rn.Next(1, 50);
    while(Contains(list, next))
    {
        next = rn.Next(1, 50);
    }
    list[i] = next;       
} 


private bool Contains(IEnumerable<int> ints, int num) 
{
    foreach(var i in ints)
    {
        if(i = num) return true;
    }
    return false;
}

edited Nov 11, 2011 at 22:44

answered Nov 11, 2011 at 20:17

Nate

30.7k24 gold badges121 silver badges187 bronze badges

3 Comments

Tevo D Over a year ago

shouldn't that be getting a new value while(Contains(next)) instead of !contains?

Tevo D Over a year ago

Adding the second function improves readability considerably.

Casperah Over a year ago

Excellent and simple, but maybe use rNumber.Next(1, 50) instead.

Collectives™ on Stack Overflow

C# populate array with unique ints No Linq or ArrayLists;

4 Answers 4

6 Comments

1 Comment

Comments

3 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

6 Comments

1 Comment

Comments

3 Comments

Your Answer

Sign up or log in

Post as a guest

Related