While loops and Graphics

[Vanessa Astle]

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!!

[Keith Lynn]

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.

[Joanne Neal]

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.

protected void generateRandomColors()
    {
        List colorList = new ArrayList();
    
        colorList.add(Color.BLUE);
        colorList.add(Color.BLUE);
        colorList.add(Color.CYAN);
        colorList.add(Color.CYAN);
        colorList.add(Color.WHITE);
        colorList.add(Color.WHITE);
        colorList.add(Color.ORANGE);
        colorList.add(Color.ORANGE);
        colorList.add(Color.MAGENTA);
        colorList.add(Color.MAGENTA);
        colorList.add(Color.YELLOW);
        colorList.add(Color.YELLOW);
        colorList.add(Color.PINK);
        colorList.add(Color.PINK);
        colorList.add(Color.GREEN);
        colorList.add(Color.GREEN);
        colorList.add(Color.RED);
        colorList.add(Color.RED);
        for(int j = 18; j < NUM_CARDS; j++)
            colorList.add(Color.RED);
 
        Collections.shuffle(colorList);
        colorArray = (Color[]) colorList.toArray(); 
    }

[Vanessa Astle]

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!

[Keith Lynn]

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.

[Vanessa Astle]

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).

[Keith Lynn]

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.

[Campbell Ritchie]

Posted by Vanessa Astle

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

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

[Vanessa Astle]

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

[Joanne Neal]

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

Color[] colours = new Color[] { Color.BLUE, Color.CYAN, Color.WHITE,
                                Color.ORANGE, Color.MAGENTA, Color.YELLOW, 
                                Color.PINK, Color.GREEN, Color.RED
                               };
for(int j = 0; j < NUM_CARDS; j++)
  colorList.add(colours[j % colours.length]);
Collections.shuffle(colorList);
colorArray = (Color[]) colorList.toArray();

[Keith Lynn]

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.

[Vanessa Astle]

Oh, really? You learn something new everyday!

[Campbell Ritchie]

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.

[Vanessa Astle]

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