Weyehn Reeves
Weyehn Reeves

Reputation: 25

Trying to find a elegant way of finding adjacent element in a 2d array in a multi-direction

I want to find the neighboring element that surrounds p. the program below doesn't print of the neighbors

I know we can use brute-force approach, like this:

array[i-1][i]
array[i-1][i-1]
array[i][i-1]
array[i+1][i]

and so on... But, I worry that it would get tedious to check every possible places of that element p. I am trying to figure out an elegant way to do this.

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

void draw_board();
int row_number, column_number;
char mark_board(char mark, int row_num, int column_num);
char find_neighborhood(char board[6][6], int i, int j);

char board[6][6] = {
    { '0','0','0','0','0','0' },
    { '0','1','2','3','0','0' },
    { '0','4','P','5','0','0' },
    { '0','6','7','8','0','0' },
    { '0','0','0','0','0','0' },
    { '0','0','0','0','0','0' }
};

int main() {
    draw_board();

    find_neighborhood(board[6][6], 3, 3); // trying to find neighbor of char p 
    
    return 0;
}

void draw_board() {
    printf("   1 2 3 4 5 6\n");
    printf("   -----------\n");
    for (int i = 0; i < 6; i++) { // rows
        printf("%i| ", i + 1);// row number to be printed
        for (int j = 0; j < 6; j++) { // columns
            printf("%c ", board[i][j]);
        }
        printf("\n");
    } 
}

char find_neighborhood(char board[6][6], int row_num, int col_num) {
    int rows = sizeof(board); // row limit 
    int columns = sizeof(board[0]) - 1; // column limit
  
    for (int j = row_num - 1; j <= row_num + 1; j++) {
        for (int i = col_num - 1; i <= col_num + 1; i++) {
            if (i >= 0 && j >= 0 && i < columns && j < rows && !(j == row_num && i == col_num)) {
                printf("The neighbor of p is: %c\n", board[j][i]);
            }
        }
    }
}

Upvotes: 2

Views: 363

Answers (2)

David C. Rankin
David C. Rankin

Reputation: 84561

In addition to the good points made by @chqlie, you have other areas that pose significant "anti-patters" or just plain "code-smells". The most vexing is your mixing of 1-based rows and columns with 0-based array indexing. In C, all indexing is 0-based. When you write functions manipulating arrays, all indexing should be 0-based. Not only does this make your code easier to follow and maintain, it removes the risk of an off-by-one error associated with mixing 1-based and 0-based indexing.

If you must take 1-based coordinates from the user, then map 1-based indexing to 0-based indexing before you call your array manipulation function. If you don't do it at the point of input, then make it clear to anyone maintaining your code that an indexing changes is occurring. A simple commented function will do, e.g.

/* convert 1-based rows/cols to 0-based indexes */
int toindex (int rc)
{
    return rc ? rc - 1 : rc;
}

and call:

    /* DANGER mixing 1-based & 0-based indexes !!! */
    find_neighborhood (board, toindex (3), toindex (3));

Don't use MagicNumbers in your code. (e.g. 6). This limits your code to a single arrays size requiring picking though all loop and array declarations and recompiling your code just to handle a change of array size. Instead:

#define ROWS 6          /* if you need a constant, #define one (or more) */
#define COLS ROWS

void draw_board (const char board[ROWS][COLS]);
void find_neighborhood (const char board[ROWS][COLS], int row, int col);

This impacts how you write you reading in draw_board() as well, e.g.

void draw_board(const char board[ROWS][COLS])
{
    fputs ("  ", stdout);               /* don't use MagicNumbers */
    for (int i = 0; i < ROWS; i++)      /* create your headings   */
        printf ("%2d", i + 1);          /* from defined constants */
    fputs ("\n   ", stdout);
    for (int i = 0; i < 2 * COLS - 1; i++)
        putchar ('-');
    putchar ('\n');
    
    for (int i = 0; i < ROWS; i++) {
        printf (" %d|", i);
        for (int j = 0; j < COLS; j++) {
            printf (j ? " %c" : "%c", board[i][j]);
        }
        putchar ('\n');
    } 
}

You have another subtle issue with your declaration of board as char board[6][6] and then your use of the const qualifier in your function parameter lists. Pointers to arrays with different qualifiers are incompatible in ISO C. C11 Standard - 6.7.6.1 Pointer declarators(p2). This is the result of array/pointer conversion on a 2D array resulting in a pointer-to-array of actual type char (*)[6]. Try it, enable full warnings with -Wall -Wextra -pedantic (or /W3 on VS)

As for a "more-elegant" ways to write find_neighborhood() your though of nested loops and bounds checks at the edges is as good as any other approach. You started out in that direction, and other than writing a set of if...else if...else if...else conditionals, it probably a good choice. Eliminating your 1-based/0-based problem it could be written as:

/* all array manipulation functions should use 0-based indexes */
void find_neighborhood (const char board[ROWS][COLS], int row, int col)
{
    printf ("\nThe neighbors of '%c' are:\n\n", board[row][col]);
    
    for (int i = row ? row - 1 : row; i <= (row < ROWS - 1 ? row + 1 : row); i++) {
        for (int j = col ? col - 1 : col; j <= (col < COLS - 1 ? col + 1 : col); j++) {
            if (i == row && j == col)
                fputs ("  ", stdout);
            else
                printf (" %c", board[i][j]);
        }
        putchar ('\n');
    }
}

Putting it altogether, you would have:

#include <stdio.h>

#define ROWS 6          /* if you need a constant, #define one (or more) */
#define COLS ROWS

void draw_board (const char board[ROWS][COLS]);
void find_neighborhood (const char board[ROWS][COLS], int row, int col);

/* convert 1-based rows/cols to 0-based indexes */
int toindex (int rc)
{
    return rc ? rc - 1 : rc;
}


int main() {
    
    const char board[ROWS][COLS] = {    /* avoid global variables   */
        { '0','0','0','0','0','0' },
        { '0','1','2','3','0','0' },    /* pointers to arrays with  */
        { '0','4','P','5','0','0' },    /* different qualifiers are */
        { '0','6','7','8','0','0' },    /* incompatible in ISO C    */
        { '0','0','0','0','0','0' },
        { '0','0','0','0','0','0' },
    };
    draw_board(board);
    
    /* DANGER mixing 1-based & 0-based indexes !!! */
    find_neighborhood (board, toindex (3), toindex (3));
}

void draw_board(const char board[ROWS][COLS])
{
    fputs ("  ", stdout);               /* don't use MagicNumbers */
    for (int i = 0; i < ROWS; i++)      /* create your headings   */
        printf ("%2d", i + 1);          /* from defined constants */
    fputs ("\n   ", stdout);
    for (int i = 0; i < 2 * COLS - 1; i++)
        putchar ('-');
    putchar ('\n');
    
    for (int i = 0; i < ROWS; i++) {
        printf (" %d|", i);
        for (int j = 0; j < COLS; j++) {
            printf (j ? " %c" : "%c", board[i][j]);
        }
        putchar ('\n');
    } 
}

/* all array manipulation functions should use 0-based indexes */
void find_neighborhood (const char board[ROWS][COLS], int row, int col)
{
    printf ("\nThe neighbors of '%c' are:\n\n", board[row][col]);
    
    for (int i = row ? row - 1 : row; i <= (row < ROWS - 1 ? row + 1 : row); i++) {
        for (int j = col ? col - 1 : col; j <= (col < COLS - 1 ? col + 1 : col); j++) {
            if (i == row && j == col)
                fputs ("  ", stdout);
            else
                printf (" %c", board[i][j]);
        }
        putchar ('\n');
    }
}

Example Use/Output

$ ./bin/board_neighbors
   1 2 3 4 5 6
   -----------
 0|0 0 0 0 0 0
 1|0 1 2 3 0 0
 2|0 4 P 5 0 0
 3|0 6 7 8 0 0
 4|0 0 0 0 0 0
 5|0 0 0 0 0 0

The neighbors of 'P' are:

 1 2 3
 4   5
 6 7 8

When you look for a "more elegant" way of doing anything, in the end you will get a fair amount of opinion built in. Let me know if you have further questions.

Upvotes: 1

chqrlie
chqrlie

Reputation: 144740

There are multiple issues in the code:

  • find_neighborhood(board[6][6], 3, 3); is incorrect: you should write this instead:

      find_neighborhood(board, 3, 3);
    
  • in find_neighborhood, the definition int rows = sizeof(board); initializes rows to the size of a pointer, not the number of rows in the matrix. You should use explicit constants or pass the dimensions as extra arguments.

  • find_neighborhood() performs many tests... But that's the core of the question.

  • row_number, column_number and board should not be global variables. It is confusing that some functions use the global variables and others take them as arguments (with the same name).

Here is a modified version:

void find_neighborhood(char board[6][6], int row, int col) {
    for (int j = max(0, row - 1); j <= min(row + 1, 6); j++) {
        for (int i = max(0, col - 1); i <= min(col + 1, 6); i++) {
            if (j != row || i != col) {
                printf("The neighbor of p is: %c\n", board[j][i]);
            }
        }
    }
}

There is an elegant way to address your objective: you can define board as an 8x8 array where the first and last rows and columns are always empty. The active part of the board has index values in the range 1..6.

Here is a modified version with this approach:

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

void draw_board(const char board[8][8]);
char mark_board(char board[8][8], char mark, int row, int col);
void find_neighborhood(char board[8][8], int row, int col);

int main() {
    int row_number, column_number;
    char board[8][8] = {
        { 0,  0,  0,  0,  0,  0,  0,  0 },
        { 0, '0','0','0','0','0','0', 0 },
        { 0, '0','1','2','3','0','0', 0 },
        { 0, '0','4','P','5','0','0', 0 },
        { 0, '0','6','7','8','0','0', 0 },
        { 0, '0','0','0','0','0','0', 0 },
        { 0, '0','0','0','0','0','0', 0 },
        { 0,  0,  0,  0,  0,  0,  0,  0 },
    };
    draw_board(board);

    find_neighborhood(board, 4, 4); // trying to find neighbors of char p 
    
    return 0;
}

void draw_board(const char board[8][8]) {
    printf("   1 2 3 4 5 6\n");
    printf("   -----------\n");
    for (int i = 1; i <= 6; i++) { // rows
        printf("%i| ", i);  // row number to be printed
        for (int j = 1; j <= 6; j++) { // columns
            printf("%c ", board[i][j]);
        }
        printf("\n");
    } 
}

void find_neighborhood(char board[8][8], int row, int col) {
    char save = board[row][col];
    board[row][col] = 0;
    for (int j = row - 1; j <= row + 1; j++) {
        for (int i = col - 1; i <= col + 1; i++) {
            if (board[j][i] != 0) {
                printf("The neighbor of p is: %c\n", board[j][i]);
            }
        }
    }
    board[row][col] = save;
}

Upvotes: 1

Related Questions