koobas.hobune.stream
Comparing each random element in an array with each other element
I don't understand why my script isn't working, and I'm having a real hard time wrapping my brain around what's actually happening.
int[] myArray = new int[numOfElements];
for (int i = 0; i < myArray.Length; i++)
{
myArray[i] = Random.Range(0, 10);
for (int k = i + 1; k < myArray.Length; k++)
{
while (myArray[i] == myArray[k])
{
myArray[i] = Random.Range(0, 10);
}
}
}
I need each element in myArray to have a random value, but there can't be any identical elements. I could define each element as a random integer and compare it to every other element individually, but besides the fact that it would be an ugly mess, the array is created in the scope of a method, and the size of the array(numOfElements) will vary when ever the function is called.
If it weren't for the repeats, this script would be doing exactly what I need.
I think that's all the information you would need to tell me what I'm doing wrong, but just in case, here is a more detailed explanation:
I have a text UI object, to which a script writes 5 strings to, selecting at random from a list of strings. In order to select 5 items from the list, I'm using an int array, labeled myArray here. The value of each element in the array is an index for the list of strings. My program successfully picks 5 strings from the list and updates the text object, but it often picks some strings more than once, which I'm fairly certain is because there are equal values in the array(identical indexes that pick the same strings).
@$$anonymous$$ossuranta gave already a perfect answer, however i just want to address some major problems in your approach:
First of all, why does your inner loop start at i+1 and go to the end? You "fill" your array from the beginning and "i" is the current element. Only values with indices smaller than "i" have already been set but you check all the elements that hasn't been set yet and you ignore those who has been set. This makes no sense.
Even when the inner for loop is fixed from 0 to "i-1", it doesn't really ensure that the picked number is none of the previous but only that it's not equal to the latest. The while loop and the inner for loop would have to switch places to actually work. So the logic would be: while not a valid number pick a new one. Each time after you picked a new one you have to check all previous ones.
Next big problem is, what if your array element count is greater than 10 ? Right, you have an infinite loop because the 11th element (index 10) can never not equal one of the previous ones since the numbers 0 to 9 have already been used for the first 10 elements.
Checking for duplicates like that is extremely inefficient and dangerous. If you have an array with exact 10 elements, the last element would have to do countless "tries" to hit a number that hasn't been picked yet. The more numbers have been assigned the less likely you will randomly pick one of the remaining number. The extreme case is when you run out of numbers in which case you will never end.
Like $$anonymous$$ossuranta showed in his answer the easiest way to distribute numbers randomly without duplicates is to just fill in the values in order and then shuffle the elements by randomly switching places of two elements.
Just as an example how to "fix" your approach. You should never use it. I just post it as reference:
int[] myArray = new int[numOfElements];
for (int i = 0; i < myArray.Length; i++)
{
int number = 0;
bool numberNotOk = true;
while (numberNotOk)
{
number = Random.Range(0,10);
numberNoOk = false; // assume we got a valid number
for(int k = 0; k < i; k++)
{
if (number == myArray[k]) // duplicate, so start over and pick a new number
{
numberNoOk = true;
break;
}
}
}
// If we got here "number" is valid so store it in the array
myArray[i] = number;
}
Well actually, tell me if I'm wrong but I think I actually caused this issue you're speaking of while making my script more appropriate for the question. In the actual script, I wasn't doing
Random.Range(0,10);
//But rather I was doing
Random.Range(0, ingredients.Count - 1);//where ingredients is a list
So if I understand both you and myself correctly, the infinite loop wouldn't happen unless numOfElements was greater than the amount of items in the list, which I wouldn't allow, right?
I don't really like your idea of how to accomplish what you want, because the while loop could just get stuck if random happens to be the same for many times in a row. One reason why it doesn't work is that it only checks once if the number at i is same as at k. After every time you randomize new number you would have to start the check again from the very beginning as now it could be same as some number that has already been checked.
I would do what you seem to be doing like this. It creates list of numbers from 0 to maxInt and then loops through the array making each elements value random value from the list and removing said element from list so it doesn't appear twice.
int numOfElements = 5;
int maxInt = 10;
int[] myArray = new int[numOfElements];
List<int> tempList = new List<int>();
for (int i = 0; i < maxInt; i++)
{
tempList.Add(i);
}
int rng;
for (int i = 0; i < myArray.Length; i++)
{
rng = Random.Range(0, tempList.Count);
myArray[i] = tempList[rng];
tempList.RemoveAt(rng);
}
If you need your array to have numbers from 0 to n. Then I would do it like this where it creates the array and then shuffles it.
void MyMethod()
{
int numOfElements = 10;
int[] myArray = new int[numOfElements];
for (int i = 0; i < numOfElements; i++)
{
myArray[i] = i;
}
ShuffleArray(myArray);
}
public static void ShuffleArray<T>(T[] arr)
{
for (int i = arr.Length - 1; i > 0; i--)
{
int r = Random.Range(0, i + 1);
T tmp = arr[i];
arr[i] = arr[r];
arr[r] = tmp;
}
}
I managed to adapt this to my project successfully and it works perfect. But there's some funky stuff going on in your ShuffleArray method that is a bit beyond me, and I'm excited to try to understand it. Thanks to everyone, happy holidays!
Oh yeah, ID$$anonymous$$ if it matters now that you helped me, but I came up with my script trying to adapt this to my project. http://stackoverflow.com/questions/23460367/comparing-elements-of-the-same-array-in-java
Your shuffle would be improved by changing the for loop limit to arr.Length-2 and the limits on the call to Random.Range(0,i+1) to Random.Range(i,arr.Length-1). This would change your shuffle into a Fisher-Yates Shuffle.
That wouldn't "improve" anything. Actually it would make it slightly worse ( in regard to performance). He already implemented the Fisher-Yates shuffle. Specifically the first pseude code example almost one-to-one.
The advantage of the reverse iteration is that you touch the array length only once in the beginning ins$$anonymous$$d of two times each iteration.
Btw: The max parameter of the Random.Range method with int parameters is "exclusive", so subtracting one would mean you never touch the last element. Random.Range(0, 5) will return a random int in the range 0 - 4 inclusive (0, 1, 2, 3, 4)
Dohhhh! your'e absolutely right. The first implementation is on the money for the Fisher-Yates shuffle from highest to lowest index.
you are initializing your array with all zeros at the start. then it iterates them and compares the current random value with the next, which is zero.
this means as long as your first random value is not zero, your equation is never true. two 3s in a row are no problem. An easier approach might be: have your index array and a list with all the numbers from 0 to the length of the array. then loop your array once and every time assign it a random value from your list from 0 to it's length and then remove it. This way, the list gets shorter and shorter and is empty at the end of the iteration, all values appearing once.
use
for(int k =0; k < i;k++)
instead of
for (int k = i + 1; k < myArray.Length; k++)
because all the next array element is 0.
Btw you should use the list, it's much easier to use.
List<int> randList = new List<int>();
int numOfElements = 5;
randList.Add(UnityEngine.Random.Range(0, 10));
for (int i = 1; i < numOfElements; i++)
{
int newNum = UnityEngine.Random.Range(0, 10);
while(randList.Contains(newNum))
{
newNum = UnityEngine.Random.Range(0, 10);
}
randList.Add(newNum);
}
