While loops and Graphics

Here is a method that I call in a drawCards() method, which is later called in the init() method of a matching game graphics program. This works as long as I don't use the while loop. But I need it to work if NUM_CARDS is greater than 18 (so the while loop seems kind of necessary...)
Can anyone let me know why this isn't working when I use the while loop, or offer a suggestion as to how I can avoid using the while loop, but still have the colors loop around if NUM_CARDS > 18?

Color[] colorArray = new Color[NUM_CARDS];

protected void generateRandomColors()
{
int colorCount = 0;
while (colorCount < NUM_CARDS)
{
for(int j = 0; j < 18; j++)
{
if (j < 2)
{
colorArray[colorCount] = Color.BLUE;
}
if (j >= 2 && j < 4)
{
colorArray[colorCount] = Color.CYAN;
}
if (j >= 4 && j < 6)
{
colorArray[colorCount] = Color.WHITE;
}
if (j >= 6 && j < 8)
{
colorArray[colorCount] = Color.ORANGE;
}
if (j >= 8 && j < 10)
{
colorArray[colorCount] = Color.MAGENTA;
}
if (j >= 10 && j < 12)
{
colorArray[colorCount] = Color.YELLOW;
}
if (j >= 12 && j < 14)
{
colorArray[colorCount] = Color.PINK;
}
if (j >= 14 && j < 16)
{
colorArray[colorCount] = Color.GREEN;
}
if (j >= 16)
{
colorArray[colorCount] = Color.RED;
}
colorCount++;
}
}
List colorList = Arrays.asList(colorArray);
Collections.shuffle(colorList);
colorArray = (Color[]) colorList.toArray();
}

Thanks so much for your time!!

I think you could make the method simpler by iterating through the array and for each element in the array generate a random color.

Remember that the Color class has a constructor that accepts a red, green, and blue component as ints between 0 and 255.

As you are converting the array to a list after you have filled it, why not use a list to start with. And as the initial values are fixed you only need a loop to add the correct number of Red elements.
So try something like this.

Believe it or not, Joanne, that was my original code, but I was having problems with my shuffling method and somewhere along the way I changed it to the code shown above. I guess that after I fixed the bug I never thought to change it back.
However, I wanted the color loop to reuse all 9 colors, as opposed to just the red one. Regardless, I will change my code to use just the arraylist as opposed to the array.
I agree with your idea on how generating a random colour could make the method much simpler, Keith. I really wanted to use those specified colours (for fear I end up with something gross, like brown) but I think that the priority here is getting different pairs of colours as opposed to mostly red cards...

Thanks so much for your help!

One suggestion I would have for that would be to create a base array containing the colors you want to use.

Then populate your array or list of colors by picking one from that array randomly.

Here is an update:
ArrayList<Color> colorArray = new ArrayList<Color>();

protected void generateRandomColors()
{
for(int j = 0; j < NUM_CARDS; j+=2)
{
int red = (int)(Math.random()*255);
int blue = (int)(Math.random()*255);
int green = (int)(Math.random()*255);
Color randomColor = new Color(red,green,blue);
colorArray.add(randomColor);
colorArray.add(randomColor);
}
Collections.shuffle(colorArray);
}

public Color getRandomColor(int i)
{
return colorArray.get(i);
}
The only problems that I am having now are that, firstly, many of the colours end up looking pretty gross. But more than anything, when I have a large number of cards, many of the colour pairs end up looking very similar to one another.
I wonder, Keith, if I would be able to do that but still select the colours in pairs (I need two of each colour, to make a match, without any old maids at the end).

Sure.

Just step through your array by 2's. Pull a random color from the array and put that color in the current position and the next position.

Then after you have it filled, you can shuffle the array.

Posted by Vanessa Astle

int red = (int)(Math.random()*255);

That is obviously a correct technique, but surely you mean "(Math.random()*256)"?

Nope, 255..Remember that it starts with 0 as opposed to 1.

If you want to restrict it to more or less equal numbers of certain colours you could do this

Originally posted by Vanessa Astle:
Nope, 255..Remember that it starts with 0 as opposed to 1.

I think Campbell's point was that the Math.random() returns a double greater than or equal to 0.0 and less than 1.0.

So Math.random()*255 will never be 255.0.

Oh, really? You learn something new everyday!

Yes, Keith was right. The largest value from Math.random() is 0.9999999999... so you multiply it by 256 to get 255.999... and when it is cast to an int you get 255.

Thanks so much Joanne! I used your suggestion, and it works perfectly! Precisely what I was looking for!