6
\$\begingroup\$

This code works but I'm not really sure if I wrote it good enough. It seems a bit vague but I can't really assess it properly. I'm particularly concerned with the xAxis variable. I don't like how I have to initialize it inside the first loop. I would rather have it next to yAxis. Is this a problem?

float yAxis = SPACE_BETWEEN_TILES;
float xAxis;

for(int i = 0; i < 3; i++) {
    xAxis = SPACE_BETWEEN_TILES;
    for(int v = 0; v < 3; v++) {
        shapeRenderer.rect(xAxis, yAxis, 10, 10);
        xAxis = TILE_WIDTH_AND_HEIGHT + xAxis;
    }
    yAxis = yAxis + TILE_WIDTH_AND_HEIGHT;
}
\$\endgroup\$
6
\$\begingroup\$

Write it this way instead:

for (int v = 0; v < 3; v++) {
    for (int h = 0; h < 3; h++) {
        float x = SPACE_BETWEEN_TILES + h * TILE_WIDTH_AND_HEIGHT;
        float y = SPACE_BETWEEN_FILES + v * TILE_WIDTH_AND_HEIGHT;
        shapeRenderer.rect(x, y, 10, 10);
    }
}

… because:

  • With your repeated addition, floating-point inaccuracies would accumulate.
  • It's easier to understand.
  • All variables are scoped as narrowly as possible.
  • i and v make a weird pair of variable names.

In addition, you should probably consider using double, as float is rarely useful.

\$\endgroup\$
7
\$\begingroup\$

I think it is mostly fine as it is. I would declare variables at the level they are actually needed, not before. Also, xAxis = TILE_WIDTH_AND_HEIGHT + xAxis; can be written as xAxis += TILE_WIDTH_AND_HEIGHT.

i and v are not very good variable names (too short, and they don't fit together). What does v even stand for? vertical? In that case, I would use vertical and horizontal.

TILE_WIDTH_AND_HEIGHT would be shorter as TILE_DIMENSION.

You could restructure it like this if you want to:

    for (int y = 0; y < 3; y++) {
        for (int x = 0; x < 3; x++) {
            shapeRenderer.rect(SPACE_BETWEEN_TILES + TILE_DIMENSION * x, SPACE_BETWEEN_TILES + TILE_DIMENSION * y, 10, 10);
        }
    }

The performance loss from transforming additions to multiplications should be fine in most cases.

\$\endgroup\$
3
\$\begingroup\$

With a little rearrangement, you can initialize xAxis both in and out of the loop, like yAxis:

float yAxis = SPACE_BETWEEN_TILES;
float xAxis = SPACE_BETWEEN_TILES;

for(int i = 0; i < 3; i++) {
    for(int v = 0; v < 3; v++) {
        shapeRenderer.rect(xAxis, yAxis, 10, 10);
        xAxis = TILE_WIDTH_AND_HEIGHT + xAxis;
    }
    yAxis = yAxis + TILE_WIDTH_AND_HEIGHT;
    xAxis = SPACE_BETWEEN_TILES;
}

This isn't really an issue, but I would probably initialize xAxis first because you write the coordinates as {x, y}, so this keeps that order:

float xAxis = SPACE_BETWEEN_TILES;
float yAxis = SPACE_BETWEEN_TILES;

for(int i = 0; i < 3; i++) {
    for(int j = 0; j < 3; j++) {
        shapeRenderer.rect(xAxis, yAxis, 10, 10);
        xAxis = TILE_WIDTH_AND_HEIGHT + xAxis;
    }

    xAxis = SPACE_BETWEEN_TILES;
    yAxis = yAxis + TILE_WIDTH_AND_HEIGHT;
}

Also, when you have two nested loops, it is traditional to use variable names i and j for your counters. If your counters represent any besides counters, you may want to name them a bit more specifically.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.