2
\$\begingroup\$

I've been working on a simple program to output values to the console as a learning project, and I stumbled across an article advising against using 2D containers, suggesting to simulate them instead with a 1D vector/container. I immediately proceeded to try to create a ridiculous class that converted input X and Y values into the correct position in the class's vector member. This class had a lot of redundant code, and eventually I realized that I was losing the value of the simplicity of the container having only one dimension by requiring two dimensions to input into the container. Instead I decided to move that math to other functions and just take a single input into the vector inside the class.

The following code is an attempt at a SimpleContainer class. I realize that I am reinventing the wheel here, but this is just a project to aid the learning process for me. I'm trying to learn C++ best practices, and things like good code layout and readability. I know that this class is missing some important optimization and features, and I think I'm doing unnecessary copying, but it would be very helpful for me to have a breakdown of what I'm doing right and wrong. This is also my first experience with using templates, but thankfully the code compiles, so I think I'm doing it right.

#include <iostream>    //Needed for std::cout and std::endl
#include <vector>      //Needed for std::vector and others

//These values are arbitrary and are only used when computing the
//screen size for things like memory allocation for the Container
#define CONSOLEWIDTH  80;
#define CONSOLEHEIGHT 25;

//The purpose of this class is to have an easy way to input variables and objects into a container,
//sometimes a large number of them at a time, and then retrieve and print them easily as well.
//Templating the class allows the input and printing of ints and chars very easily
//You could input objects, but some of the functions may not work with the code as written
template <class myType>
class SimpleContainer
{
public:
    //I think there are other ways to do the following but I don't know them
    SimpleContainer(int containerSize, myType defaultValue)
    {
        classVector.resize(containerSize);

        for(int i = 0; i < containerSize; i++)
        {
            classVector[i] = defaultValue;
        }
    }

    //Simply looks at the classVector size and returns (used for iterating over it)
    int getSize();

    //The setValue function sets the value at specified container position
    void setValue(int containerPosition, myType inputValue);

    //I feel like I'm missing a reference here but <myType>& threw an error
    void inputEntireVector(std::vector<myType> inputVector);

    //You have to compute the X and Y positions outside of this function if you want
    //to simulate a 2D matrix
    myType getValue (int containerPosition);

    //Prints the entire contents of the container starting from vector.begin()
    //I think this will only work with ints and chars, and with ints over 9 it will
    //mess up the alignment of the console view
    void printContainer();

private:
    std::vector<myType> classVector;
};

template<class myType>
int SimpleContainer<myType>::getSize()
{
    return classVector.size();
}

template<class myType>
void SimpleContainer<myType>::setValue(int containerPosition, myType inputValue)
{
    classVector.erase(classVector.begin() + containerPosition);

    //vector.insert() takes for its third argument a value for the number of times to
    //insert the input value
    int numInputCopies = 1;
    classVector.insert(classVector.begin() + containerPosition, numInputCopies, inputValue);
}

template<class myType>
void SimpleContainer<myType>::inputEntireVector(std::vector<myType> inputVector)
{
    classVector.swap(inputVector);
}

template<class myType>
myType SimpleContainer<myType>::getValue(int containerPosition)
{
    return classVector[containerPosition];
}

template<class myType>
void SimpleContainer<myType>::printContainer()
{
    for(int i = 0; i < classVector.size(); i++)
    {
        std::cout << classVector[i];
    }
}

//Runs some basic interactions with the Container
void sampleContainerInterfacing();

int main()
{
    sampleContainerInterfacing();
    return 0;
}

void sampleContainerInterfacing()
{
    //Setting integers for the view width and height to input into Container
    //Uses them immediately to do the math to figure out Container size
    int width  = CONSOLEWIDTH;
    int height = CONSOLEHEIGHT;
    int containerSize = width*height;

    SimpleContainer<int> mySimpleContainer(containerSize, 0);

    //Outputs one console screen worth of 0's
    mySimpleContainer.printContainer();

    std::cout << mySimpleContainer.getSize() << std::endl;

    //Defining these variables to aid readability of the 2D matrix math
    int position;
    int posY = 5;
    int posX = 7;
    //The position in the container equals the width * the desired y position
    //plus the desired x position, -1 because it starts counting with 0
    position = width * posY + posX - 1;

    mySimpleContainer.setValue(position, 5);
    std::cout << mySimpleContainer.getValue(position) << std::endl;

    //Now contains the input variable
    mySimpleContainer.printContainer();
}
\$\endgroup\$
  • \$\begingroup\$ If you are simulating a 2D vector with a 1D vector, you should still be able to use the 2D syntax m[1][1]. \$\endgroup\$ – Martin York Jul 27 '12 at 16:59
3
\$\begingroup\$

Interface:

Even though you are implementing the 2D array as a large 1D array. The user of the array is still expecting to be able to index into the array using the normal array notation.

 Matrix  m(2,3);   // 2D array size 2,3 (default constructed).

 m[1][2] = 4;

You do not want to move the calculation of the position outside the container. If you do this you are exposing implementation details of how the container is built and requiring the user to understand things about the layout of your container. This binds your hands for future modifications and thus breaks encapsulation.

It is technically possible to use the [] to do this. But it is not obvious for beginners (as operator[] can only take 1 parameter) and thus we will use () to simulate the 2D accesses (as this makes the code easier to write).

// The usage of the code will look like this.
// It is not a massive change and people will understand it easily.
// Also the user of the array does not need to know how it is layed out.
m(1,2) = 4;

So your first pass of the interface should look like this:

template <class myType>
class SimpleContainer
{
       std::vector<myType>  data;
       std::size_t          xSize;
       std::size_t          ySize;
    public:
    SimpleContainer(std::size_t xs, std::size_t ys, myType const& defaultValue = myType())
        : data(xs * ys, defaultValue)
        , xSize(xs)
        , ySize(ys)
    {}

    myType&       operator()(std::size_t x, std::size_t y)       {return data[y*xSize + x];}
    myType const& operator()(std::size_t x, std::size_t y) const {return data[y*xSize + x];}
};

Notice I have written two versions of operator(). Both return references to the internal members but one version returns a const reference and is also marked as const.

The normal version:

 myType&       operator()(std::size_t x, std::size_t y)

Allows read/write access to the elements in the array. Because it returns a reference any mutations I do to this object will be reflected in the container copy.

m(4,5) = 6; // Will modify the container version to be 6

The const version:

myType const& operator()(std::size_t x, std::size_t y) const

Allows read only access to the object. This is useful as it allows you to pass your container as a const reference to functions and still access the data. And the compiler will guarantee that the access to the data does not modify it.

If you want to add the ability to do m[1][2] to your container then read this article: https://stackoverflow.com/questions/3755111/how-do-i-define-a-double-brackets-double-iterator-operator-similar-to-vector-of/3755221#3755221

\$\endgroup\$
  • \$\begingroup\$ Thank you. This was very succinct and completely explained the logical failure on my part. Also as a result of your post, I now understand operator overloading in classes in C++ which hadn't really clicked before. Much appreciated! \$\endgroup\$ – bazola Jul 30 '12 at 10:54
3
\$\begingroup\$

Simple Review of code

In this pass I will do a simple review of the code you have written.
I think there are some problems with the design of your interface but I will deal with those completely separately so this part of the review is solely on the code you have written and assumes that the interface is good.

You don't need to comment why you are including header files.

#include <iostream>    //Needed for std::cout and std::endl
#include <vector>      //Needed for std::vector and others

Don't use macros.

#define CONSOLEWIDTH  80;
#define CONSOLEHEIGHT 25;

These macros have no scope (are all scope depending on how you look at it). Thus you are polluting the namespace. It would be preferable to use int const value. Personally I would make them static members of the class. This allows you to define them inline.

class sampleContainerInterfacing
{
    static int const ConsoleWidth  = 80;
    static int const ConsoleHeight = 25;

The vector already has a constructor that initializes all values:
http://www.sgi.com/tech/stl/Vector.html

Also note sizes are usually defined via std::size_t as this can never be negative.

    //I think there are other ways to do the following but I don't know them
    SimpleContainer(int containerSize, myType defaultValue)
    {
        classVector.resize(containerSize);

        for(int i = 0; i < containerSize; i++)
        {
            classVector[i] = defaultValue;
        }
    }

Try this:

   SimpleContainer(std::size_t size, myType defaultValue)
        : classVector(size, defaultValue)
   {}

This function is simple enough that I would inline it.

    int getSize();

Same comment as constructor. Use std::size_t to indicate a position in the container.

    void setValue(int containerPosition, myType inputValue);

I am not sure I agree with your implementation of setValue(). The call to erase() removes tha value from the array. This will cause the vector to copy all the elements (above containerPosition) down one position (that could be a huge number of copies if the type is not a POD. Then you insert a value back into the location which causes all the elements (from containerPosition) to copied back up 1 position.

template<class myType>
void SimpleContainer<myType>::setValue(int containerPosition, myType inputValue)
{
    classVector.erase(classVector.begin() + containerPosition);

    //vector.insert() takes for its third argument a value for the number of times to
    //insert the input value
    int numInputCopies = 1;
    classVector.insert(classVector.begin() + containerPosition, numInputCopies, inputValue);
}

Personally I would just overwrite the value in the container:

void setValue(int containerPosition, myType inputValue) {classVector[containerPosition] = inputValue;}

Personally I would also pass by (const) reference. The error was probably caused because you did not match the declaration and definition (ie you needed to add the & to both locations).

    //I feel like I'm missing a reference here but <myType>& threw an error
     template<class myType>
    void SimpleContainer<myType>::inputEntireVector(std::vector<myType> inputVector)
    {
        classVector.swap(inputVector);
    }

But you are using swap() on the internal array (which is a valid technique) but requires you to make the copy first so that you don't destroy the original array. What it comes down to is that you need to copy the values from the input into the array. There are two ways to do this.

  • Assignment: (Pass parameter by const reference then use assignment)
  • Copy and Swap: (Pass parameter by value (thus doing an implicit copy) then call swap()

Same comment as above: Parameter should be std::size_t and its simple enough to put inline.

    //You have to compute the X and Y positions outside of this function if you want
    //to simulate a 2D matrix
    myType getValue (int containerPosition);

Sure: This is fine. But I would pass the stream the the values are printed on as a parameter (thus you are not hard coding it to std::cout).

    //Prints the entire contents of the container starting from vector.begin()
    //I think this will only work with ints and chars, and with ints over 9 it will
    //mess up the alignment of the console view
template<class myType>
void SimpleContainer<myType>::printContainer()
{
    for(int i = 0; i < classVector.size(); i++)
    {
        std::cout << classVector[i];  // Note no space between elements.
    }
}

But if you are going to do that then you should also provide an overload of operator<<(std::ostream& stream, SimpleContainer& data) that calls this method.

Traditionally it is more usual to provide iterators that allow the user to control how much of the array they want to print. Then you can copy the whole array with:

std::copy(con.begin(), con.end(), std::ostream_iterator<type>(std::cout, ", "));

Thus I would write the print like this:

template<class myType>
inline void SimpleContainer<myType>::printContainer(std::ostream& stream)
{
    for(int i = 0; i < classVector.size(); i++)
    {
        stream << classVector[i];  // Note no space between elements (like original)
    }
    // Or you can use std::copy()
    // std::copy(classVector.begin(), classVector.end(),
    //            std::ostream_iterator<type>(stream, ""/*No Space*/));
}

template<class myType>
inline std::ostream& operator<<(std::ostream& stream, impleContainer<myType> const& data)
{
    data.printContainer(stream);
    return stream;
}
\$\endgroup\$
  • \$\begingroup\$ This post was very helpful, thank you! I didn't realize a couple of things until reading your post. First that vector::erase made copies of the information, and second, that you can define static values inline in classes. I appreciate your time looking at my code. \$\endgroup\$ – bazola Jul 30 '12 at 10:55

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.