1

I used the following formula to create the colors for the different rows in breakout. But I find myself repeating the if function 5 times to get 5 different colors. I was wondering if there is a more efficient way to do this without using so many if functions?

    for (int j = 0; j < COLS; j++)
    {
        brick = newGRect(6 + (brickwd + brickspc) * j, 40 + (brickht + brickspc) * i, brickwd, brickht);
        setFilled(brick, true);
        add(window, brick);
        if (i == 0)
        {
            setColor(brick, "black");
        }
        else if (i == 1)
        {
            setColor(brick, "red");
        }
        else if (i ==2)
        {
            setColor(brick, "orange");
        }
        else if (i == 3)
        {
            setColor(brick, "green");
        }
        else
        {
            setColor(brick, "blue");
        }   
    }

3 Answers 3

5

Yes. Usually there's a better approach to do something specially when you find yourself repeating code or checking every possible case manually to do something different in each situation.

I prefer the idea of storing all the color names I want in an array of strings, specify the size of that array and maybe hard-code it (you can calculate it either) and sense the changing the color of a brick mainly depends on i, I would do something like

setColor(myColors[i % arrayLength], brick);

where myColors is the array of strings that contains the color names, arrayLength is the length of the array (and I'm taking the remainder of dividing i by this length to ensure that my index is always valid) and brick is the brick that you're setting its color.

1
  • Thanks @kareem, I did think of this approach but did not know how to create an array in this way.
    – Lucas
    Commented Nov 16, 2014 at 16:27
1

Definitely, your if series must be improved.

I did it similarly to the @Kareem's approach. I also made an array of colours with the length of the number of rows. Then in the same loop where the brick rows instantiated, I add colour feature to each brick.

0

This is just a thought, because I feel the answers so far have been somewhat more difficult than my solution. They are probably cleaner but I was thinking along simple lines...

Thanks though to Kareem, I figured this out because you mentioned break.

I used switch/case, similar to your ifs but cleaner looking, no arrays. Mine didn't work either. But I saw an example online for cases and after each case it used break. I just added a break to each case....

    switch(i) //this instantiates but the coloring doesnt work. need a new set of rectangles for each line to set each individual color.
    {
        case 0: setColor(brick, "RED");
            break;
        case 1: setColor(brick, "ORANGE");
            break;
        case 2: setColor(brick, "YELLOW");
            break;
        case 3: setColor(brick, "GREEN");
            break;
        case 4: setColor(brick, "BLUE");
            break;

When I didn't add the breaks, ALL the bricks were the LAST color. The break kept each case from changing all the bricks' colors each time.

Not as fancy as an array but I guess the point is, breaks were needed to get this to instantiate like the CS50 example.

0

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .