2
\$\begingroup\$

I want to generate an array where each row and column needs to have a random generated amount (between 2 and total size of dimension -1) of true values.

Currently I am running each row then columns and check if it validates. If it does not it regenerates and has to do another full loop to check if all rows and columns validate.

/**
 * Procedurally populates 2D array so that each line and row contains between 1 and .Length true values.     
 */
public static boolean[][] generateTargetNumbers(int width, int height) {

    boolean[][] targetNumbers = new boolean[width][height];

    // keep looping trough rows and columns to validate and regenerate if necessary
    boolean finished = false;
    while (!finished) {
        finished = true;

        // check columns
        for (int x = 0; x < width; x++) {
            if (generateTargetColumn(x, width, height, targetNumbers)) {
                finished = false;
            }
        }

        // check rows
        for (int y = 0; y < height; y++) {
            if (generateTargetRow(y, width, height, targetNumbers)) {
                finished = false;
            }
        }
    }
    return targetNumbers;
}

/**
 * Checks if the column has a valid amount of targets and regenerates the column if nececary.
 * @param x
 * @param width
 * @param height
 * @param targetNumbers
 * @return returns true if the column is regenerated.
 */
private static boolean generateTargetColumn(int x, int width, int height, boolean[][] targetNumbers) {
    int minTargetCount = 2;
    int maxTargetCount = height - 1;

    // count targets on this column
    int currentCount = 0;
    for (int y = 0; y < height; y++) {
        if (targetNumbers[x][y]) currentCount++;
    }

    if (currentCount >= minTargetCount && currentCount <= maxTargetCount) return false;

    // Column needs regenerating

    // Reset column to false
    for (int y = 0; y < height; y++) {
        targetNumbers[x][y] = false;
    }

    // Amount of targets for this column.
    int targetCount = Random.nextInt(maxTargetCount - minTargetCount + 1) + minTargetCount;

    while (targetCount > 0) {
        int index = Random.nextInt(height);
        if (!targetNumbers[x][index]) {
            targetNumbers[x][index] = true;
            targetCount--;
        }
    }

    return true;
}

/**
 * Checks if the row has a valid amount of targets and regenerates the column if nececary.
 * @param y
 * @param width
 * @param height
 * @param targetNumbers
 * @return returns true if the row is regenerated.
 */
private static boolean generateTargetRow(int y, int width, int height, boolean[][] targetNumbers) {
    int minTargetCount = 2;
    int maxTargetCount = height - 1;

    // count targets on this row
    int currentCount = 0;
    for (int x = 0; x < width; x++) {
        if (targetNumbers[x][y]) currentCount++;
    }

    if (currentCount >= minTargetCount && currentCount <= maxTargetCount) return false;

    // Row needs regenerating

    // Reset column to false
    for (int x = 0; x < width; x++) {
        targetNumbers[x][y] = false;
    }

    // Amount of targets for this row.
    int targetCount = Random.nextInt(maxTargetCount - minTargetCount + 1) + minTargetCount;

    while (targetCount > 0) {
        int index = Random.nextInt(height);
        if (!targetNumbers[index][y]) {
            targetNumbers[index][y] = true;
            targetCount--;
        }
    }

    return true;
}

For my needs (5x5 to 9x9 arrays) it usually does it in just 2 loops so that is great. But the randomness is biased to a larger amount of true values since it adds additional true values to the rows when it encounters a invalid column. I would like to have more control over the amount/percentage of true values.

Besides that, the near identical bottom 2 methods are tickling my OCD senses. I'm sure someone knows a nice way of combining these and make it look cleaner.

\$\endgroup\$
  • \$\begingroup\$ You say between 2 and total size of dimension -1 of true values, but the comment says between 1 and .Length true values. Which one is correct? \$\endgroup\$ – Raimund Krämer May 28 '18 at 11:15
  • \$\begingroup\$ @RaimundKrämer The comment is excluding 1 and total length. For a array of size 5 it would be 2, 3 or 4 true values. Sorry for being vague, the actual code where I assign minTargetCount and maxTargetCount is correct. \$\endgroup\$ – Madmenyo May 28 '18 at 21:09
4
\$\begingroup\$
boolean finished = false;
while (!finished) {
    finished = true;

    // check columns
    for (int x = 0; x < width; x++) {
        if (generateTargetColumn(x, width, height, targetNumbers)) {
            finished = false;
        }
    }

    // check rows
    for (int y = 0; y < height; y++) {
        if (generateTargetRow(y, width, height, targetNumbers)) {
            finished = false;
        }
    }
}

We're working with booleans here. I can directly assign the result to the variable...

boolean finished = false;
while (!finished) {
    finished = true;

    // check columns
    for (int x = 0; x < width; x++) {
        finished = !generateTargetColumn(x, width, height, targetNumbers) && finished;
    }

    // check rows
    for (int y = 0; y < height; y++) {
        finished = !generateTargetRow(y, width, height, targetNumbers) && finished;
    }
}

There's a lot of negations in here, so maybe we can flip the variable's meaning and get clearer code?

boolean busy = true;
while (busy) {
    busy = false;

    // check columns
    for (int x = 0; x < width; x++) {
        busy = generateTargetColumn(x, width, height, targetNumbers) || busy;
    }

    // check rows
    for (int y = 0; y < height; y++) {
        busy = generateTargetRow(y, width, height, targetNumbers) || busy;
    }
}

Sure, but "busy" is not really what we're looking for, after all, you say that

@return returns true if the column/row is regenerated.

So we should change the name to "gridChanged" or something like that.

boolean gridChanged = true;
while (gridChanged) {
    gridChanged = false;

    // check columns
    for (int x = 0; x < width; x++) {
        gridChanged = generateTargetColumn(x, width, height, targetNumbers) || gridChanged;
    }

    // check rows
    for (int y = 0; y < height; y++) {
        gridChanged = generateTargetRow(y, width, height, targetNumbers) || gridChanged;
    }
}

But now you have this weird thing where at the start, you say "I had changes", which seems wrong... and you always want to go in the loop at the start. So convert it to a do-while:

boolean gridChanged = true;
do {
    gridChanged = false;

    // check columns
    for (int x = 0; x < width; x++) {
        gridChanged = generateTargetColumn(x, width, height, targetNumbers) || gridChanged;
    }

    // check rows
    for (int y = 0; y < height; y++) {
        gridChanged = generateTargetRow(y, width, height, targetNumbers) || gridChanged;
    }
} while(gridChanged);
\$\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.