I have to use magic numbers to display values in the correct position in an array. How can I get rid of it?

Sorry if the code is too long. I just wanted to give a detailed description of the situation I am facing

I'm coding a game called Battle Ship. The following code is a simpler version, I did eliminate all the unnecessary logic, cause I just want to indicate the problem with the magic numbers.

Here are my struct and enum

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
// the dimension of the 10 + 2 game grid for detection of the neighboring ship 
// (its usage comes later in the game, not at this stage)
const int SIZE = 12;

// the number of warships
const int NBSHIP = 5;

string ships[NBSHIP] = {"Carrier", "Cruiser", "Destroyer", "Submarine", "Torpedo" };

// available warships and their size on the grid
enum Ship {
  CARRIER=5,
  CRUISER=4,
  DESTROYER=3,
  SUBMARINE=3,
  TORPEDO=2,
  NONE=0
};

// the different states that a grid cell can take and their display
enum State {
  HIT='x',
  SINK='#',
  MISS='o',
  UNSHOT='~'
};

// a grid cell
// the ship present on the space
// the state of the box
struct Cell {
  Ship ship;
  State state;
};

// the player with
// his name
// his score to determine who lost
// his game grid
struct Player {
  string name;
  int score;
  Cell grid[SIZE][SIZE];
};

// the coordinates of the cell on the grid
// its column from 'A' to 'J'
// its line from 1 to 10
struct Coordinate {
  char column;
  int line;
};

// the placement of the ship on the grid
// its coordinates (E5)
// its direction 'H' horizontal or 'V' vertical from the coordinate
struct Placement {
  Coordinate coordi;
  char direction;
};


Basically, at the beginning of the game, I have to initialize a grid with the appropriate state for each cell (in this case, UNSHOT and NONE). Then I have to display the grid and start placing the ships. The weird thing here is I have to use "magic numbers" to place the ships in the correct position according to the player's input. But I don't even know why I need it as well as how to get rid of it.

Utilization of magic number appears in placeShip function.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
void initializeGrid (Cell aGrid[][SIZE])
{
    for (int i = 0; i < SIZE - 2; i++)
    {
        for (int j = 0; j < SIZE - 2; j++)
        {
          aGrid[i][j].ship = NONE;
          aGrid[i][j].state = UNSHOT;
        }
    }
}

void displayGrid(Player aPlayer)
{
    cout << endl;
    cout << setw(10) << aPlayer.name << endl;

    // Letter coordinates
    char a = 'A';
    cout << "  "; 
    for (int i = 0; i < SIZE - 2 ; i++)
    {
        cout << " " << char (a+i);
    }
    cout << endl;

    // Number coordinates
    for (int i = 0; i < SIZE - 2; i++)
    {
        // Player
        if(i + 1 >= 10)
            cout << i + 1;
        else
            cout << " " << i + 1;

        for (int j = 0; j < SIZE - 2; j++)
        {
            if (aPlayer.grid[i][j].ship) // Check if there are ships
                cout << " " << (aPlayer.grid[i][j].ship);
            else
                cout << " " << char (aPlayer.grid[i][j].state);
        }
        cout << endl;
    }
}

void placeShip(Cell aGrid[][SIZE], Placement aPlace, Ship aShip)
{
    if (aPlace.direction == 'h' || aPlace.direction == 'H')
    {
        for (int i = 0; i < aShip; i++) // To change the value of a cell according to the size of the boat
        {
          // Utilization of magic number
          aGrid[aPlace.coordi.line - 9][aPlace.coordi.column + i - 1].ship = aShip; // Horizontal so we don't change the line
        }
    }
    else if (aPlace.direction == 'v' || aPlace.direction == 'V')
    {
        for (int i = 0; i < aShip; i++)
        {
          // Utilization of magic number
          aGrid[aPlace.coordi.line + i - 9][aPlace.coordi.column - 1].ship = aShip; // Vertical so we don't change the column
        }
    }
}

void askPlayerToPlace(Player& aPlayer)
{
    Ship battleships[NBSHIP] = { CARRIER, CRUISER, DESTROYER, SUBMARINE, TORPEDO};

    for (int i = 0; i < NBSHIP; i++)
    {
        string stringPlace;
        string stringShip = ships[i];
        Ship aShip = battleships[i];
        string inputDirection;
        Coordinate coordi;

        cout << "\n" << aPlayer.name << ", time to place your carrier of length " 
        << to_string(battleships[i]) 
        << " (" << ships[i] << ")\n" 
        << "Enter coordinates (i.e. B5): ";
        cin >> stringPlace;
        coordi = { stringPlace[0], int(stringPlace[1]) - 48 };

        cout << "Direction: ";
        cin >> inputDirection;
  
        Placement aPlace = {
            coordi,
            inputDirection[0]
        };

        placeShip(aPlayer.grid, aPlace, aShip);
        displayGrid(aPlayer);
    }
}

int main()
{
    Player aPlayer;

    cout << "Player's name: ";
    getline (cin, aPlayer.name);

    initializeGrid(aPlayer.grid);
    displayGrid(aPlayer);

    askPlayerToPlace(aPlayer);

    return 0;
}
Last edited on
most of the time magic numbers are eliminated simply by giving them a name as a constant or enum (if you have several related ones) or the like. Its that simple:

aGrid[aPlace.coordi.line - 9] becomes:
aGrid[aPlace.coordi.line - name_that_describe_what_this_is]

It "may" be possible to rewire the logic so you don't need the value -- I don't know at a glance. This frequently becomes a chain reaction that ends up being a do-over on the whole project, whereas just giving it a name and a comment explaining what it does leaves working code alone without the special number making the code ugly.
Last edited on
Topic archived. No new replies allowed.