11
\$\begingroup\$

This program prints out a 10x10 square and uses only bit operations. It works well, but I don't like that global array.

Can you tell me if the code is proper or not?

#include <iostream>

const std::size_t HEIGHT = 10;
const std::size_t WIDTH = 10;

char graphics[WIDTH/8][HEIGHT];

inline void set_bit(std::size_t x, std::size_t y)
{
    graphics[(x) / 8][y] |= (0x80 >> ((x) % 8));
}

void print_screen(void)
{
    for (int y = 0; y < HEIGHT; y++)
    {
        for (int x = 0; x < WIDTH/8+1; x++)
        {
            for (int i = 0x80; i != 0; i = (i >> 1))
            {
                if ((graphics[x][y] & i) != 0)
                    std::cout << "*";
                else
                    std::cout << " ";
            }
        }
            std::cout<<std::endl;
    }
}

int main()
{
    for(int x = 0; x < WIDTH; x++)
    {
        for(int y = 0; y < HEIGHT; y++)
        {
            if(x == 0 || y == 0 || x == WIDTH-1 || y == HEIGHT-1)
                set_bit(x,y);
        }
    }
    print_screen();
    return 0;
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Why not pass the array as an argument and then make it local to main? \$\endgroup\$ – idoby Aug 31 '13 at 19:14
  • \$\begingroup\$ because in the set_bit I have to change it. I tried using as a reference, but it said I couldn't. \$\endgroup\$ – mitya221 Aug 31 '13 at 19:19
  • 1
    \$\begingroup\$ I'd use a 2D std::array as it's easier to handle, adding on to what busy_wait is suggesting. \$\endgroup\$ – Jamal Aug 31 '13 at 19:33
  • \$\begingroup\$ it's a solution for an exercise (from a c++ book). I had to use bits :) \$\endgroup\$ – mitya221 Aug 31 '13 at 19:41
  • \$\begingroup\$ std::vector<bool> is optimized for space and since this is C++ you should prefer to use vector over an array anyway. \$\endgroup\$ – idoby Aug 31 '13 at 19:44
12
\$\begingroup\$

That global array is indeed not good. You'll need to pass around an array, but you shouldn't do it with a C-style array. Doing that will cause it to decay to a pointer, which you should avoid in C++. If you have C++11, you could use std::array, which will be set at an initial size. But if you don't have C++11, and also want to adjust the size, use an std::vector. You can also compare the two here. Either way, you'll be able to pass any of them around nicely, and it's something you should be doing in C++ anyway.

To match your environment, the following code does not utilize C++11.

I'll use std::vector here, but this can be done with other STL storage containers. Here's what a 2D vector would look like:

std::vector<std::vector<T> > matrix; // where T is the type

This type does look long, and you may not want to type it out each time. To "shorten" it, you can use typedef to create an alias (which is not a new type):

typedef std::vector<std::vector<T> > Matrix;

With that, you can use this type as such:

Matrix matrix;

and create the 2D vector of a specific size.

However, this is where the syntax gets nasty (especially lengthy). It's not set to a specific size, so you can just push vectors into it to increase the size. For a fixed size (using your size and data type), you'll have something like this:

std::vector<std::vector<char> > matrix(HEIGHT, std::vector<char>(WIDTH));

This can be made shorter by having another typedef to serve as a dimension of the matrix. This will also make it a little clearer what the vector means in this context.

typedef std::vector<char> MatrixDim;

It is then applied to the Matrix typedef:

typedef std::vector<MatrixDim> Matrix;

The 2D initialization will then become this:

Matrix matrix(HEIGHT, MatrixDim(WIDTH));

Now you can finally use this in main() and pass it to the other functions. Before you do that, you'll need a different loop counter type. With an STL storage container, you should use std::size_type. With std::vector<char>, specifically, you'll have:

std::vector<char>::size_type;

You can use yet another typedef for this:

typedef MatrixDim::size_type MatrixDimSize;

Here's what the functions will look like with the changes (explanations provided). I've also included some additional changes, which are also explained. The entire program with my changes applied and produces the same output as yours.

setbit():

inline void set_bit(Matrix& matrix, MatrixDimSize x, MatrixDimSize y)
{
    matrix[(x) / 8][y] |= (0x80 >> ((x) % 8));
}
  • An additional parameter of type Matrix is added. The matrix is passed in by reference and modified within the function.
  • The std::size_t parameters were replaced with the MatrixDimSize type.

print_screen():

void print_screen(Matrix const& matrix)
{
    for (MatrixDimSize y = 0; y < HEIGHT; y++)
    {
        for (MatrixDimSize x = 0; x < WIDTH/8+1; x++)
        {
            for (int i = 0x80; i != 0; i >>= 1)
            {
                std::cout << (((matrix[x][y] & i) != 0) ? '*' : ' ');
            }
        }

        std::cout << "\n";
    }
}
  • A parameter of type Matrix is added. The matrix is passed in by const&, which is necessary as the function displays the matrix but does not modify it. It's also cheaper to pass it this way as opposed to copying (passing by value).
  • MatrixDimSize is added for the loop counter types.
  • The if/else is replaced with an equivalent ternary statement.
  • A newline is done with "\n" as opposed to std::endl. The latter also flushes the buffer, which is slower. You just need the former.
  • i = (i >> 1) is shortened to i >>= 1.

Main():

int main()
{
    Matrix matrix(HEIGHT, MatrixDim(WIDTH));

    for (MatrixDimSize x = 0; x < WIDTH; x++)
    {
        for (MatrixDimSize y = 0; y < HEIGHT; y++)
        {
            if (x == 0 || y == 0 || x == WIDTH-1 || y == HEIGHT-1)
            {
                set_bit(matrix, x, y);
            }
        }
    }

    print_screen(matrix);
}
  • Both matrix vector typedefs are applied.
  • MatrixDimSize is added for the loop counter types.
  • The matrix is passed to and modified only by set_bit().
    • It is passed to print_screen() and is not modified.
\$\endgroup\$

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.