2
\$\begingroup\$

I've been messing around with arrays as I'm learning them currently in my book and they gave the idea of a chess board using for loops alongside multidimensional arrays to give them individual values which you can therefore print out.

However, I thought of the old console RPG games from way back and wanted to simply add a border around the array so it prints any 0 || 19 value as an X. The code does work however, is there a way of making it more efficient by using if statements or by created a switch statement that reads the current loop variable and change that array value to a 1 for fill or leave it 0 to blank the "tile".

#include <iostream>

using namespace std;

int main()
{
    int rpgLevelGrid[20][20];

    //Co-od decleration 
    for (int i = 0; i < 20; i++) 
    {
        for (int j = 0; j < 20; j++)
        {
            switch (i) 
            {
            case 0: case 19:
                rpgLevelGrid[i][j] = 1;
                break;
            default:
                rpgLevelGrid[i][j] = 0;
                break;
            }

            switch (j)
            {
            case 0:
                rpgLevelGrid[i][j] = 1;
                break;
            case 19:
                rpgLevelGrid[i][j] = 1;
                break;
            }
        }
    }

    for (int i = 0; i < 20; i++)
    {
        for (int j = 0; j < 20; j++)
        {
            switch (rpgLevelGrid[i][j])
            {
            case 1:
                cout << "X";
                break;
            default:
                cout << "-";
            }
        }
        cout << endl;
    }
}
\$\endgroup\$
5
\$\begingroup\$

switch vs. if

In

        switch (rpgLevelGrid[i][j])
        {
        case 1:
            cout << "X";
            break;
        default:
            cout << "-";
        }

do not use switch when all you need is a regular if. Instead do

        if (rpgLevelGrid[i][j])
        {
            cout << "X";
        }
        else
        {
            cout << "-";
        }

or even better do

        cout << (rpgLevelGrid[i][j] ? "X" : "-");

You could also fill the grid with the characters to print unless there is reason not to do so. Then you only do

        cout << rpgLevelGrid[i][j];

A third option for the output is to define a symbol table like

const char symbols[] = "-X";

and use it like

        cout << symbols[rpgLevelGrid[i][j]];

Initialisation

Also our inner init loop is not consistent. you conditionally init with 1 on i but conditionally overwrite the previously initialized 0 with 1 on j. Wwhy not init all with 0 and overwrite the borders? That is less complex. So

        switch (i) 
        {
        case 0: case 19:
            rpgLevelGrid[i][j] = 1;
            break;
        default:
            rpgLevelGrid[i][j] = 0;
            break;
        }

        switch (j)
        {
        case 0:
            rpgLevelGrid[i][j] = 1;
            break;
        case 19:
            rpgLevelGrid[i][j] = 1;
            break;
        }

becomes

        rpgLevelGrid[i][j] = 0;
        if (i==0 || i==19 || j==0 || j==19)
        {
            rpgLevelGrid[i][j] = 1;
        }

Number literals

Still there are some magic numbers. We now define

const int dim = 20;

and replace all literals in the code like

int rpgLevelGrid[dim][dim];

for (int i = 0; i < dim; i++) 

case dim-1:

or

        if (i==0 || i==dim-1 || j==0 || j==dim-1)

Functions

Finally we pull out some code from main and provide two functions for init and print and end up in

#include <iostream>

using namespace std;

const int dim = 20;

void init(int grid[dim][dim])
{
    for (int i = 0; i < dim; i++) 
    {
        for (int j = 0; j < dim; j++)
        {
            grid[i][j] = 0;
            if (i==0 || i==dim-1 || j==0 || j==dim-1)
            {
                grid[i][j] = 1;
            }
        }
    }
}

void print(int grid[dim][dim])
{
    const char symbols[] = "-X";
    for (int i = 0; i < dim; i++)
    {
        for (int j = 0; j < dim; j++)
        {
            cout << symbols[grid[i][j]];
        }
        cout << endl;
    }
}

int main()
{
    int rpgLevelGrid[dim][dim];

    init(rpgLevelGrid);
    print(rpgLevelGrid);
} 

That is a resonable result for a beginner.

\$\endgroup\$
2
\$\begingroup\$

Another way to fill the array is to first ensure it is initialized outside any for-loop to contain only blank tiles, and then to just add the border. For example:

int rpgLevelGrid[dim][dim] = {}; // Initialize with all-zeroes

for (int i = 0; i < dim; i++) {
  rpgLevelGrid[i][0] = 1;
  rpgLevelGrid[i][dim - 1] = 1;
  rpgLevelGrid[0][i] = 1;
  rpgLevelGrid[dim - 1][i] = 1;
}

The above code does not contain any conditional statements, and this generally makes code faster. It can be improved further by splitting the for loop into multiple loops for the top row, middle section and bottom row, so that memory is written to sequentially, and to avoid writing to the four corner tiles twice.

\$\endgroup\$
  • 1
    \$\begingroup\$ Alternative way to avoid writing to the corner tiles twice each, without needing separate loops: start the loop at 1 instead of 0, and offset the top and right (or bottom and left) indices by -1. I think the cure is worse than the disease here, so I'm not really recommending this! I'm not sure how important the sequential access is, unless you're very unlucky with your cache alignment. \$\endgroup\$ – Toby Speight Sep 25 '18 at 13:13

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.