Thread: what is wrong with this function

Originally Posted by cooper1200

even hard coding them in rather than using a pointer doesnt work. and the printf statement doesn't print any thing.

I compiled and ran this program:

Code:

#include <stdbool.h>
#include <stdio.h>
#include <string.h>

#define COL_MAX 2
#define ROW_MAX 2
#define MESSAGE 10

bool check_king(char board[][ROW_MAX + 1][MESSAGE], int current_player, int x_coordinate, int y_coordinate)
{
    //char *p_king_label = current_player ? "B" : "W";
    //printf("%s\n", p_king_label);
 
    //return ((board[x_coordinate][y_coordinate] == 'W') || (board[x_coordinate][y_coordinate] == 'B')) ? true : false;
    //check this works properly once we have a king if not do strcat outside of the return by adding current player as a parameter
    printf("w%sW", board[x_coordinate][y_coordinate]);
    return ((strcmp(board[x_coordinate][y_coordinate], "W") == 0) ||
           ((strcmp(board[x_coordinate][y_coordinate], "B") == 0))) ? true : false;
}

int main(int argc, char *argv[])
{
    char board[COL_MAX + 1][ROW_MAX + 1][MESSAGE] = {{"B", "w"}, {"b", "w"}};
    if (check_king(board, 0, 0, 0))
    {
        printf("\ncheck_king returned true\n");
    }
    else
    {
        printf("\ncheck_king returned false\n");
    }
    return 0;
}

It compiled without any errors or warnings, and I received this output:

Code:

wBW
check_king returned true

So... what's wrong with this function? Who knows?

In the future, I won't bother doing this. I'll just ask you to refer to my forum signature.

By the way, this:

Code:

return ((strcmp(board[x_coordinate][y_coordinate], "W") == 0) ||
       ((strcmp(board[x_coordinate][y_coordinate], "B") == 0))) ? true : false;

can be simplified to:

Code:

return (strcmp(board[x_coordinate][y_coordinate], "W") == 0) ||
       (strcmp(board[x_coordinate][y_coordinate], "B") == 0);

I understand the concern about not getting the order of precedence right, but when you're using two sets of parentheses where one set will do, you really need to start thinking of reducing your parentheses usage: at some point they begin to obfuscate rather than illuminate the code.

The result of the || operator is already a boolean value. Granted, it is technically an int with the value of either 1 or 0, but that doesn't make any difference.

Put it this way: whenever you see an expression of this form:

Code:

x ? true : false

ask yourself: is x already a boolean value? If so, simplify to just x. Otherwise, there's probably a comparison you can introduce to turn it into a boolean value.

Likewise, if it is of this form:

Code:

x ? false : true

you can simplify to !x or introduce a comparison.

Originally Posted by laserlight

Likewise, if it is of this form:

Code:

x ? false : true

you can simplify to !x or introduce a comparison.

so instead of

Code:

position = p_position == NULL ? false :  true;
return position;

i can have

Code:

position = !p_position == NULL;
return position;

or do away with the position variable and just return !p_position == NULL;
coop

I would write:

Code:

return p_position != NULL;

That's fine.