2
\$\begingroup\$

I'm writing a myVector class for a class I'm taking. I don't believe I'm "strong" with smart pointers right now, so that's my question. This is just an excerpt from a much larger class that I've completed.

Is there a way to use the shared pointer and dynamically allocate the size, so that I can use the [] notation with that pointer? I've been looking on Stack Overflow and don't see anything referencing this particular usage. My next question/conclusion would then be that it's best to stick with raw pointers in the case where I want to use array notation.

#include <iostream>
//#include <memory>

class myVector
{
private:
    int sz;
    int * array;

public:
    myVector(int a, int b)
    {
        sz = a;
        array = new int[sz];
        for(int i(0); i < sz; ++i)
            array[i] = b;
    }
    ~myVector(){ delete [] array; };
    int size() const
    {
        return sz;
    }
    int &operator[](const int num)
    {
        return array[num];
    }
};

int main()
{
    myVector testing(10, 6);
    for(int i(0); i < testing.size(); ++i)
    {
        testing[i] = testing[i] * i;
        std::cout << testing[i] << std::endl;
    }
    return 0;
}
\$\endgroup\$
1
\$\begingroup\$

In the following, I tried to collect the options you have for your custom vector class, and summarize their pros and cons. But before that I'd suggest to use generic programming when you write your vector class, i.e.

template<typename T>
class myVector
{
     size_t sz;
     T* array;

     //all the rest
};

Note two things here: Instead of the int *, I've wrote T*, where T is the type you specify in the program via myVector<int>, say. This you can hold elements of arbitrary type and you do not need to copy the whole class once you need a double type.

Further, I've used the type size_t for the size of the vector, which is the type often used in C++ to denote sizes of objects (in fact, it's just a large unsigned int type).


Choosing the right storage type:

  • First, and most naturally, you can use an STL container inside, preferably std::vector: this is one example of the RAII way to do it, which basically means that you usually don't have to care for the memory management by yourself. Although from the first it sounds a bit redundant to include a std::vector in your custom vector class, it actually can be rather useful, for instance when you want to add arithmetics to your vector.

    template<typename T>
    class myVector
    {
         std::vector<T> array;
    };
    

    This has several advantages: visibly, you don't need a variable sz anymore, as the std::vector already manages the size. Next -- and this is an important thing -- as your array becomes correctly copied, moved, assigned and destructed, your class will perform these operations correctly as well.

    In the following, let us consider the copy operation as an example: in the above approach, when one copies the class, the new class contains a copy of the original std::vector.

  • Using a raw pointer as in your OP. This is nowadays a discouraged way to do it, just because you have to manually care for the memory management. Once you invoke array = new int[sz], array is a pointer to some allocated memory on the heap, i.e. it's so to say something outside of the class, and the class itself must take care that this memory is correctly released for further usage when it is destructed. Thus you need to write a destructor, and therefore -- according to the rule of three(five) -- also copy constructors, assignment operators (, move constructors).

    Consider again the copy process. The copied myVector just copies the raw pointer, such that now two pointers point to the same memory region. However, these two pointers don't know about each other. So, it becomes a hard task to prevent one of the classes to delete the memory when it goes out of scope, as then the other class will become a dangling pointer: it points to a destructed and thus invalid object. In order to avoid this, you'd need to count how many pointers point to a given object, and call the delete only if the last pointer goes out of scope (i.e. when nobody needs the object anymore).

  • Shared pointers exactly do this: they count how many pointers point to a given object which has been allocated on the heap.

    template<typename T>
    class myVector
    {
         std::shared_ptr<T> array;
         // construct via array= std::make_shared( new T[sz], std::default_delete<T[]>() );
         // this is necessary so that "delete[]" is called, an not "delete".
    
         //or better
         std::shared_ptr<std::vector<T> > array;
    };
    

    By using them, you get almost the same behaviour as by using a raw pointer but have a lot of advantages. Most importantly, you again are on the RAII track, you don't need to care for all the default functions and let the compiler generate them. A disadvantage is that a shared_ptr is usually less performant than a raw pointer, but this is often taken into account in favor of a correct code.

    Regarding the example, copying works fine: the newly copied class only contains the pointer to the object and not the object itself, which is an advantage when the arrays are large and do not need to be changed in the copy. In the copy process, the shared_ptr increases its so-called reference count, such that it knows when exactly to call delete.

  • Finally, there is another smart pointer, namely std::unique_ptr. It addresses the above problem in a completely different way: it simply doesn't allow copying, so that there is always one pointer which does the memory management. It's also said that the unique_ptr owns the object. Other, non-owning pointers are allowed, but these should never do any memory-related operations (--never call delete).

    It is possible to use it here and it could also be reasonable, but be aware that your class won't possess automatically generated copy and assignment operators. This is probably not intended.


Summary: as written above, I'd use the std::vector<T> alternative, and depart from it only for a good reason. Only then, I'd use the shared_ptr alternative. I would never use the raw pointer array.

\$\endgroup\$
  • \$\begingroup\$ When you build a generic vector you can't use T* array;. This is because when you expand the vector using functions like reserve() you will be constructing all the elements (which is not what you want to do, if you are not going to use an element you don't want to pay the cost of initializing and destroying the element). So a generic container must used an underlying storage type of char* array; \$\endgroup\$ – Martin York Nov 26 '14 at 14:20
  • \$\begingroup\$ The use of std::size_t should be discouraged. Several prominent members of the standards committee is has said that it was a mistake on their part to use it for sizes in the standard (unfortunately it is a mistake they can't take back). \$\endgroup\$ – Martin York Nov 26 '14 at 14:23
  • \$\begingroup\$ Using a RAW pointer is discourage in general. BUT this is the one case where it is actually a good idea to potentially use RAW pointers. There are two types of memory management. Smart pointers and containers. These are both low level memory management tools and using RAW pointers in their implementation is a valid technique. It is when you move to higher level constructs (business logic) that you should avoid RAW pointers like the plague. \$\endgroup\$ – Martin York Nov 26 '14 at 14:26
  • \$\begingroup\$ Shared pointer is not really a valid option here. When you do a copy of an array are you really expecting the underlying data to be shared between the two copies. You have to consider usage semantics. The usage semantics of an array are not really compatible with a shared pointer. \$\endgroup\$ – Martin York Nov 26 '14 at 14:30
  • \$\begingroup\$ Prefer using unique_ptr over shared_ptr (most situations sharing is not what you want) and it is also much more efficient. Use shared_ptr only if you truly need to share the underlying memory as now you start running into problems of memory cycles and you have to consider std::weak_ptr \$\endgroup\$ – Martin York Nov 26 '14 at 14:33
1
\$\begingroup\$

OK. Good start.

Rule of three/Five

Definition

But you have not considered the rule of three/five.

When you define a class (or struct) there are three (potentially 5 in C++11) member functions that the compiler will generate for you. These work well under normal situations but when you start working with RAW pointers and memory management the semantics do not mash well (know as the shallow copy problem).

The automatically generated methods are:

  • Copy Constructor
  • Assignment Operator
  • Destructor
  • Move Constructor (C++11)
  • Move Assignemnt (C++11)
  • Default Construcor (not counted in the rule of three/five)

The rule of three. If you do memory management in any of the first three methods above then the compiler generated versions will not be adequate (and you will need to do work).

The rule of five is if you invoke the rule of three then you can probably get some benefit by using the move semantics but the default action should work (move will be disabled by default if you have followed the rule of three).

Why it breaks your code.

Consider this:

{
    myVector    a(5,2);
    myVector    b(a);       // Copy constructor invoked.
}

What is happening here. The compiler generated copy constructor will look like this:

myVector(myVector const& rhs)
    : sz(rhs.sz)
    , array(rhs.array)
{}

So now both object are sharing the same pointer. So far this is not a problem (though probably not what the user expected). But when a and b go out of scope at the end of the function they will both call delete on the array member (double delete is undefined behavior).

The same rule applies for assignment operator.

{
    myVector   a(5,6);
    myVector   b(7,2);

   a = b; // assignment operator.

The compiler generated version would be

myVector& operator=(myVector const& rhs)
{
    sz    = rhs.sz;
    array = rhs.array;  // Here you have leaked memory.
    return *this;
}

In addition to the memory leak, you also suffer from the double delete problem again.

Solution one

Remove the ability to copy and assign your array

class myVector
{
    myVector(myVector const&)             = delete;
    myVector& operator=(myVector const&)  = delete;
};

// If you are using an old compiler C++03 or earlier.
// You will have to resort to an old trick.
class myVector
{
    // Make the two methods private.
    // and don't provide an implementation.
    myVector(myVector const&);
    myVector& operator=(myVector const&);
};

This is a ligitamate solution for a wide sub-set of tasks. But I don't think it applies to to your vector expample. As a user of a vector I would expect to be able to copy it.

Solution two

Take the time to create a copy.

myVector::myVector(myVector const& rhs)
    : sz (rhs.sz)
    , array(new int[rhs.sz])
{
    for(int loop = 0;loop < sz; ++loop)
    {
        array[loop] = rhs.array[loop];
    }
}

// For the assignment operator use the copy and swap idium
myVector& myVector::operator=(myVector rhs)
                           // ^^^^^^^^- Note he pass by value
{                          // This makes an implicit copy using the copy constructor
    rhs.swap(*this);       // swap the current object with the parameter (a copy)
                           // When the rhs values goes out of scope it deletes
    return *this;          // the memory that used to be in this object.

}

// Swap should be exception safe.
void myVector::swap(myVector& copy) noexcept
{
    std::swap(sz,     copy.sz);
    std::swap(array,  copy.array);
}

// May as well provide the swap utility while we are here.
void swap(myVector& lhs, myVector& rhs)
{
    lhs.swap(rhs);
}
\$\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.