Having problems placing ships in my Battleship program in C

Question

I've been struggling with this for about 2 hours now. For some reason instead of placing the characters over the ~ that make up my board the characters are placed in random spots that are not the cordinates I enter. Hopefully someone canhelp guide me in the right direction.

void place_ships(int board[ROWS][COLS])
{

    int ship_type = 1, ship_length = 0, row = 0, col = 0, direction = 0, count = 0;

    for (ship_type = 1; ship_type < 6; ship_type++)
    {
                    if (ship_type == 1)
                    {
                        printf("Please enter where you would like to place your Carrier (5 spots).\n\n");
                        ship_length = 5;
                    }
                    else if (ship_type == 2)
                    {
                        printf("Please enter where you would like to place your Battleship (4 spots).\n\n");
                        ship_length = 4;
                    }
                    else if (ship_type == 3)
                    {
                        printf("Please enter where you would like to place your Submarine (3 spots).\n\n");
                        ship_length = 3;
                    }
                    else if (ship_type == 4)
                    {
                        printf("Please enter where you would like to place your Cruiser (3 spots).\n\n");
                        ship_length = 3;
                    }
                    else if (ship_type == 5)
                    {
                        printf("Please enter where you would like to place your Destroyer (2 spots).\n\n");
                        ship_length = 2;
                    }

        printf("Choose the direction of your ship. (0 is vertical, 1 is horizontal)\n\n");
        scanf("%d",&direction);

            printf("Please choose ROW number.\n");
            scanf("%d",&row);
            printf("Please choose COLUMN number.\n");
            scanf("%d",&col);

            if (direction == 0)
            {

                    for (count = 0; count < ship_length; count++)
                    {
                            if (ship_type == 1)
                            {
                                board[row+count][col] = 'c';
                                print_board (board, row, col);
                            }
                            else if (ship_type == 2)
                            {
                                board[row+count][col] = 'b';
                            }
                            else if (ship_type == 3)
                            {
                                board[row+count][col] = 's';
                            }
                            else if (ship_type == 4)
                            {
                                board[row+count][col] = 'r';
                            }
                            else if (ship_type == 5)
                            {
                                board[row+count][col] = 'd';
                            }
                    }
            }

            if (direction == 1)
            {
                    for (count = 0; count < ship_length; count++)
                    {
                            if (ship_type == 1)
                            {
                                board[row][col+count] = 'c';
                            }
                            else if (ship_type == 2)
                            {
                                board[row][col+count] = 'b';
                            }
                            else if (ship_type == 3)
                            {
                                board[row][col+count] = 's';
                            }
                            else if (ship_type == 4)
                            {
                                board[row][col+count] = 'r';
                            }
                            else if(ship_type == 5)
                            {
                                board[row][col+count] = 'd';
                            }
                        }
                }
        }
}

The print board function is:

void print_board (char board[ROWS][COLS], int num_rows, int num_cols)
{
    int row_index = 0, col_index = 0;

    for (row_index = 0; row_index < num_rows; row_index++)
    {
        for (col_index = 0; col_index < num_cols; col_index++)
        {
            printf ("%c ", board[row_index][col_index]);
        }
        putchar ('\n');
    }
}

okay so this is what I changed it to. Do I need to define a struct in my header? Again I'm a big beginner...

void place_ships(int board[ROWS][COLS])
{

    int ship_type = 0, ship_length = 0, row = 0, col = 0, direction = 0, count = 0;
    char * typestr[] = { "Ship type 0", "Carrier", "Battleship", "Submarine", "Cruiser", "Destroyer" };
    char* tag = "0cbsrd"; 

    for (ship_type = 0; ship_type < 6; ship_type++)
    {
        ship_length = 7 - ship_type;
        prinf("Please enter where you would like to place %s (%s) spots).\n\n",typstr[ship_type], ship_length);

        printf("Choose the direction of your ship. (0 is vertical, 1 is horizontal)\n\n");
        scanf("%d",&direction);

        printf("Please choose ROW number.\n");
        scanf("%d",&row);
        printf("Please choose COLUMN number.\n");
        scanf("%d",&col);

        if (direction == 0)
        {
            board[row+count][col] = tag[ship_type];
        }
        else
        {
            board[row][col+count] = tag[ship_type];
        }
    }
}

Answer 1

A simple switch statement instead of the if-else list:

switch (ship_type)
{
case 1:
    board[row+count][col] = 'c';
    break;
case 2:
    board[row+count][col] = 'b';
    break;
case 3:
    board[row+count][col] = 's';
    break;
case 4:
    board[row+count][col] = 'r';
    break;
case 5:
    board[row+count][col] = 'd';
    break;
}

Actually, a better solution might be like this:

char ship_symbols[] = { 'c', 'b', 's', 'r', 'd' };
board[row+count][col] = ship_symbols[ship_type - 1];  /* -1 because array indexes start at 0 */

Answer 2

Some improvement suggestions:

• don't use if() stuff if you can calculate something.

  I.e., instead of

                  if (ship_type == 1)
                  {
                      printf("Please enter where you would like to place your Carrier (5 spots).\n\n");
                      ship_length = 5;
                  }
                                  ...
                  else if (ship_type == 5)
                  {
                      printf("Please enter where you would like to place your Destroyer (2 spots).\n\n");
                      ship_length = 2;
                  }
  

  better use

  ship_length = 7 - ship_type;
  printf("Please enter where you would like to place your %s (%s spots).\n\n", typestr[ship_type], ship_length);
  

  provided you have defined somewhere

   char * typestr[] = { "Ship type 0", "Carrier", "Battleship", "Submarine", "Cruiser", "Destroyer" };
  

  and instead of

  if (ship_type == 1)
  {
      board[row+count][col] = 'c';
      print_board (board, row, col);
  }
  ...
  

  use

  if (direction == 0) {
      board[row+count][col] = tag[ship_type];
  } else {
      board[row][col+count] = tag[ship_type];
  }
  print_board (board, row, col);
  

  with

  char* tag = "0cbsrd";
  

• Add some output after reading in the data. This shows you if you really have read the correct data.

• Are your array boundaries correct?

Answer 3

I see a few potential problems that may screw it up for you:

scanf is a bit tricky since it will also leave the newline char in the buffer after reading out the number. You may want to use fgets() instead to read from the keyboard, then use sscanf on the string to extract the number (or atoi).

when you write into the matrix you need to make sure the ship fits so after the user has entered the coordinate and the direction you need to check if the ship fits e.g.

if ( direction == vertical )
{
  ok = ( row + ship_length < ROWS ) // assuming row index [0, ROWS-1]
}
else 
{
  ok = ( col + ship_length < COLS )
}

and yes like somebody else already mentioned please use enums for constant values e.g.

enum directions { vertical, horizontal };