Efficient and Elegant Solution for Filling a Matrix with 0s and 1s and Setting Surrounding Cells to 0 when Necessary

Question

I need to fill a matrix with random 0s or 1s, but if a cell contains a 1, then all of the cells around it (8 cells in total) should be set to 0. I have attempted to implement this code, but I am unsure if there is a simpler way to do it. Can you please suggest a more efficient or elegant solution? Thank you!

As a beginner, I tried to implement my code in this way:

for (int i = 0; i < 5; i++) {
    for (int j = 0; j < 5; j++) {
        a[i][j] = rand() % 2;
        if (a[i][j] == 1) {
            a[i-1][j-1] = 0; 
            a[i-1][j]   = 0; 
            a[i-1][j+1] = 0;
            a[i][j-1]   = 0; 
            a[i][j+1]   = 0;
            a[i+1][j+1] = 0; 
            a[i+1][j]   = 0; 
            a[i+1][j+1] = 0;
        }
    }
}

I don't think what you have done is unreasonable, except that it overruns the bounds of your arrays when a `1` is generated in a corner or on an edge, which will be fairly frequent. You need to avoid that. — John Bollinger, Dec 21 '20 at 19:23
@JohnBollinger My code works, I know, but I think it's redundant. I'm sure there's a simpler, cleaner way to do this. Is THIS my question, but anyway I will solve the error you reported to me, thanks! — Chiara Tumminelli, Dec 21 '20 at 19:28
Suppose 2 adjacent cells contain `1`? which decides? If you overwrite one of them with `0`, what about **its** neighbours? I think you can only solve this with a 2-stage process, creating a second array from the first. — Weather Vane, Dec 21 '20 at 19:38
As John mentioned, the approach itself isn't bad, though consider making your code more modular by refactoring the `if` condition code into a separate method called something like `markSurroundingCellsAsZero` or whatever is most appropriate. This would help make your code slightly easier to read and debug, especially if you plan on adding a lot more to your code. Maybe even a few comments describing the functionality. Also, consider using variable names more representative of what they are doing, `a` doesn't tell much. Your approach seems good, but some of these may help in the long run :) — Sal, Dec 21 '20 at 19:38
If the code works and you're looking for advice on improving it, [codereview.se] is the appropriate place. But see https://codereview.meta.stackexchange.com/questions/5777/a-guide-to-code-review-for-stack-overflow-users first. — Barmar, Dec 21 '20 at 19:39
@ChiaraTumminelli I agree with John that there are array bounds errors. When `i=0` and `j=0`, you assign `a[i-1][j-1] = 0`, which accesses `a[-1][-1]`. — Barmar, Dec 21 '20 at 19:40
One way to make it cleaner is putting the `{` on the next line, so that the `{` and the corresponding `}` is on the same column. — 12431234123412341234123, Dec 21 '20 at 19:49
If you want the cells that contain `1` to be uniformly distributed, then you need a different approach. The algorithm you're using will be much more likely to place a `1` in the last row of the matrix. — r3mainer, Dec 21 '20 at 20:00

score 1 · Answer 1 · answered Dec 21 '20 at 19:56

You probably should do it in two steps because, otherwise, you can write a 1 again on a cell you have previously erased. Moreover you must avoid writing outside the matrix. Here is an example :

int main()
{
    // First fills the matrix :
    int a[5][5] = {0};
    printf("Random Matrix :\n");
    for (int i = 0; i < 5; i++){
        for (int j = 0; j < 5; j++){
            a[i][j] = rand() % 2;
            printf("%d ", a[i][j]);
        }
        printf("\n");
    }
    // Next filters it :
    printf("---------\nFiltered Matrix :\n");
    for (int i = 0; i < 5; i++){
        for (int j = 0; j < 5; j++){
            if (a[i][j] == 1) {
                if (i > 0) {
                    a[i-1][j]              = 0; 
                    if (j > 0) a[i-1][j-1] = 0;
                    if (j < 4) a[i-1][j+1] = 0;
                }
                if (i < 4) {
                    a[i+1][j]              = 0; 
                    if (j < 4) a[i+1][j+1] = 0;
                }
                if (j > 0) a[i][j-1] = 0; 
                if (j < 4) a[i][j+1] = 0; 
               
            }
            printf("%d ", a[i][j]);
        }
        printf("\n");
    }
    return 0;
}

Output

Random Matrix :
1 0 1 1 1 
1 0 0 1 1 
0 1 0 1 1 
0 0 0 0 0 
1 0 1 1 0 
---------
Filtered Matrix :
1 0 1 0 1 
0 0 0 0 0 
0 1 0 1 0 
0 0 0 0 0 
1 0 1 0 0

Writing 1 on a previously erased cell is not necessarily a problem, because the previously-written 1 associated with the erasure would itself be erased. The final result thus still satisfies the criteria for the exercise. There's nothing inherently wrong with doing the job as you describe, but I don't find it even as clean as the OP's original, and they are asking for something cleaner. — John Bollinger, Dec 21 '20 at 20:35
The OP codes generated many errors (*stack smashing detected*) when I did test it so, actually, I wouldn't say that it was 'clean'. Moreover, with the OP technique it remains 4 '1' in the matrix after processing while, with the two steps technique, it remains 7 instead and the requirements are satisfied in both cases. But I admit that keeping a maximum '1' in the matrix was not a requirement of the OP. Thanks for your comment anyway. — dspr, Dec 21 '20 at 22:25

anotherOne · Answer 2 · 2020-12-21T20:52:46.127

Apart for accessing the array out of bounds, your program is fine. Here's a more general approach working for a range greater than 1 cell

#include <stdio.h>
#include <stdlib.h>

#define ROW 5
#define COL 5
#define RAN 1 // range

int main() {

    int a[ROW][COL];
    
    for (int i = 0; i < ROW; i++) {
    
        for (int j = 0; j < COL; j++) {
        
            if( (a[i][j] = rand() % 2) ) {
                
                for(int k = i-RAN; k <= i+RAN && k < ROW; k++) {
                    
                    for(int l = j-RAN; l <= j+RAN && l < COL; l++) {
                        
                        if( k >= 0 && l >= 0 && !(k == i && l == j)) {
                            
                            a[k][l] = 0;
                        }
                    }
                }
            }
        }
    }
    
    for (int i = 0; i < ROW; i++) {
    
        for (int j = 0; j < COL; j++) {
            
            printf("%d ", a[i][j]);
        }
        puts("");
    }
    
    return 0;
}

score 1 · Accepted Answer · answered Dec 28 '20 at 18:30

There is an easier way to do this: you need to use the Manhattan Formula. So, code can be:

int distance = sqrt(pow((x1-x2),2) + pow((y1-y2),2))

where x1, y1 are the coords of my cell and x2, y2 are the coords of the other cells containing 1. If distance < 1 you cannot set the cell to 1. So, you can create a matrix containing the coords of all cells containing 1 and do the check on the entire matrix before setting a cell to 1.

John Bollinger · Answer 4 · 2020-12-21T20:28:22.593

I don't think there is a simpler or cleaner way to do the job. Your code is clear and about as brief as can be hoped. There are alternatives that have somewhat nicer properties, such as not biasing the appearance of 1s toward the edges and especially the bottom right of the matrix, but that's a different criterion.

As I wrote in comments, you do need to avoid overrunning the bounds of your arrays (which will make your code a bit more complex), and of course it's unnecessary to add 0 to your random number.

You might be able to do it more cleverly by encoding the matrix in some other way, such as bitwise in an array of unsigned char, but that would not be simpler, and probably not cleaner, either.

One pretty clean way to solve the overrun issue might be to declare your array two elements larger in each dimension, and use one-based indexing for the real elements. That would provide a margin on all sides of the real data such that all your writes are in bounds for the actual arrays, even if some go outside the meaningful data. That is:

int a[7][7] = {0};  // a[1][1] ... a[5][5] will contain the real data

for (int i = 1; i < 6; i++) {
    for (int j = 1; j < 6; j++) {
        a[i][j] = rand() % 2;
        if (a[i][j] == 1) {
            a[i-1][j-1] = 0; 
            // ...
        }
    }
}

You consider the actual matrix you're generating as only those elements having 1 <= i && i <= 5 and 1 <= j && j <= 5. It doesn't matter, then, that you use an index of i - 1 or j + 1, or such, because those are valid for all i and j of the real data.

Geek Wisdom · Answer 5 · 2020-12-21T21:50:22.447

A few thoughts

Your code is a loop inside a loop. For 2D arrays you can consider doing it all in just one loop. For small arrays ie: 1-5 it really doesn't matter. For larger arrays your algorithm will be faster if you calculate i,j as a function of the array. See: Map a 2D array onto a 1D array

Also you could name your variables 'a' 'i',j something like row,col or something more meaningful depending on the code's indented purpose.

The other part that confuses me a bit. Assuming your array is 5x5 and also assuming your end goal is to ensure every '1' always has 8 neighbors of zero, once the first random number generate produces a '1' and forces the 8 neighbors to zero as many as 4 neighbors no longer need to be updated. (i,j+1)/(i+1,j)/(i+1,j+1)/(i-1,j). As noted by @Weather Vane you are overwriting your work this maybe where you get that 'redundant' feeling.

#define MAX_ARRAY 5


int in_bounds(int row,int col)
{
if (row >= MAX_ARRAY) return 0;
if (col >= MAX_ARRAY) return 0;
return 1;
}

void print_matrix (int matrix[MAX_ARRAY][MAX_ARRAY],int size)
{

for (int i=0;i<size;i++)
 {
 for (int j=0;j<size;j++)
  {
  fprintf(stdout,"%d",matrix[i][j]);
//  fprintf(stdout,"%d,%d ",i,j);
  }
 fprintf(stdout,"\n");

 }
}

int main()
{
//for stack overflow: https://stackoverflow.com/questions/65398722/is-there-a-cleaner-way-to-write-this-c-code/65399364#65399364
   srand(time(0));
    int matrix[MAX_ARRAY][MAX_ARRAY];
     memset(matrix, -1, sizeof(int) * MAX_ARRAY * MAX_ARRAY);
  
    for (int counter=0;counter<MAX_ARRAY * MAX_ARRAY;counter++)
      {
      int col=counter / MAX_ARRAY;
      int row=counter % MAX_ARRAY;

      if (matrix[row][col] == -1)
        {
        matrix[row][col] = rand() %2;
        if (matrix[row][col] == 1)
         {
            if (in_bounds(row,col+1)) matrix[row][col+1] = 0;
            if (in_bounds(row+1,col)) matrix[row+1][col] = 0;
            if (in_bounds(row+1,col+1)) matrix[row+1][col+1] = 0;
            if (in_bounds(row-1,col)) matrix[row-1][col] = 0;
            if (in_bounds(col,row-1)) matrix[col][row-1] = 0;
            if (in_bounds(row-1,col-1)) matrix[row-1][col-1] = 0;
            if (in_bounds(row-1,col+1)) matrix[row-1][col+1] = 0;
            if (in_bounds(row+1,col-1)) matrix[row+1][col-1] = 0;
        }
       }

  }
print_matrix(matrix,MAX_ARRAY);
}

score 0 · Answer 6 · answered Dec 21 '20 at 20:35

Your code has a problem: you write to matrix cells with coordinates outside the range 0..4.

Here is a modified version:

for (int i = 0; i < 5; i++) {
    for (int j = 0; j < 5; j++) {
        a[i][j] = 0;
        if (rand() % 2) {
            for (int ii = i - 1; i <= i + 1; i++) {
                if (ii >= 0 && ii < 5) {
                    for (jj = j - 1; j <= j + 1; j++) {
                        if (jj >= 0 && jj < 5)
                            a[ii][jj] = 0;
                    }
                }
            }
            a[i][j] = 1;
        }
    }
}

Efficient and Elegant Solution for Filling a Matrix with 0s and 1s and Setting Surrounding Cells to 0 when Necessary

6 Answers6