1

I have a Container class that is meant to store a vector of shared pointers. Whenever an item is appended to the Container, I want it to assume ownership of that item. In other words, when the Container is deconstructed, all of the elements inside it should also be deconstructed.

template <typename T>
class Container
{
private:
    const std::vector<std::shared_ptr<T>> vec_;


public:
    void append(std::shared_ptr<T> item)
    {
        vec_.push_back(std::move(item));
    }

    void printElements()
    {
        for (int i = 0; i < vec_.size(); i++) { std::cout << vec_[i] << std::endl; }
    }
};

int main(int argc, char** argv) {

    std::unique_ptr<Container<std::string>> c = std::make_unique<Container<std::string>>();
    c->append(std::make_shared<std::string>("hello"));

    return 0;
}

The problem is, I get the following error on vec_.push_back(std::move(item)).

No matching member function for call to 'push_back'

I'm not sure why this error is occurring.

Carpetfizz
  • 8,707
  • 22
  • 85
  • 146
  • Why not use a `unique_ptr`? – cigien May 10 '20 at 04:34
  • 1
    Your vec_ is const. Seems to me that would forcefully remove or disable any method, such as push_back, which tries to modify the const vector. – Gabriel Staples May 10 '20 at 04:37
  • @GabrielStaples thanks, removing `const` did the trick. I thought `const` only protected against reassignment of `vector_` but allowed for it to be mutated? – Carpetfizz May 10 '20 at 04:38
  • @cigien this was a toy example to debug another class which requires `shared_ptr` – Carpetfizz May 10 '20 at 04:39
  • Since that's it, I'll make it an answer. – Gabriel Staples May 10 '20 at 04:40
  • 2
    "I thought const only protected against reassignment of vector_ but allowed for it to be mutated?" No, `const` here applies to the contents of the vector. The vector isn't a pointer, but what you're talking about could be done with pointers, by making the pointer itself `const` instead of the contents of what it points to `const`. Also, `const` and `mutable` are opposites. One undoes the other. You cannot have both in effect at the same time. By definition something constant is immutable (unchangeable), and something mutable is non-constant. – Gabriel Staples May 10 '20 at 04:43
  • @Carpetfizz, I massively expanded my answer to show how to do what you were thinking. Take a look. – Gabriel Staples May 10 '20 at 23:00
  • Also keep in mind: don't create pointers or smart pointers unnecessarily. If you can create something statically, that's always the way to go, even when using pointers (just use statically-allocated pointers to statically-allocated data). Your private vector object, `vec_`, is created statically at compile-time, which is good. It will then automatically use dynamic memory allocation under-the-hood during run-time, as necessary, as it fills up when appending to it. Use explicit dynamic memory allocation, including smart pointers, only where necessary. ... – Gabriel Staples May 10 '20 at 23:08
  • ... Smart pointers are a means of better managing dynamic memory allocation, to *avoid* accidentally creating memory leaks by forgetting to properly use the `delete` keyword, and should only be used when dynamic memory allocation is required. Again, prefer static memory allocation, including for variables/objects of C++ Standard Library container types (which may then rely on dynamic memory allocation under-the-hood as they are used and as they grow), wherever possible. – Gabriel Staples May 10 '20 at 23:13

1 Answers1

3

Original answer:

Your std::vector, vec_ is const. Seems to me that would forcefully remove or disable any method, such as push_back(), which tries to modify the const vector.


Additions added 10 May 2020:

I think it's worth expounding upon this comment from @Carpetfizz in the comments under his question, too:

@GabrielStaples thanks, removing const did the trick. I thought const only protected against reassignment of vector_ but allowed for it to be mutated?

My response:

No, const here applies to the contents of the vector. The vector isn't a pointer, but what you're talking about could be done with pointers, by making the pointer itself const instead of the contents of what it points to const. Also, const and mutable are opposites. One undoes the other. You cannot have both in effect at the same time. By definition, something constant is immutable (unchangeable), and something mutable is non-constant.

And how might one make a pointer const but not the contents of what it points to?

First, consider the original code (with some minor modifications/fixes I did to it):

Run it yourself online here: https://onlinegdb.com/SyMqoeU9L

1) cpp_template_const_vector_of_smart_ptrs_test_BEFORE.cpp:

#include <iostream>
#include <memory>
#include <vector>

template <typename T>
class Container
{
private:
    // const std::vector<std::shared_ptr<T>> vec_; // does NOT work 
    std::vector<std::shared_ptr<T>> vec_;          // works!


public:
    void append(std::shared_ptr<T> item)
    {
        vec_.push_back(std::move(item));
    }

    void printElements()
    {
        for (int i = 0; i < vec_.size(); i++) 
        { 
            // Don't forget to dereference the pointer with `*` in order to 
            // obtain the _contens of the pointer_ (ie: what it points to),
            // rather than the pointer (address) itself
            std::cout << *vec_[i] << std::endl; 
        }
    }
};

int main(int argc, char** argv) 
{
    std::unique_ptr<Container<std::string>> c = std::make_unique<Container<std::string>>();

    c->append(std::make_shared<std::string>("hello"));
    c->append(std::make_shared<std::string>("world"));

    c->printElements();

    return 0;
}

Output:

hello
world

And here's the new code demonstrating how to make a constant pointer to a non-const vector. See my comments here, and study the changes:

Run it yourself online here: https://onlinegdb.com/HyjNx-L5U

2) cpp_template_const_vector_of_smart_ptrs_test_AFTER.cpp

#include <iostream>
#include <memory>
#include <vector>

template <typename T>
class Container
{
private:
    // const std::vector<std::shared_ptr<T>> vec_; // does NOT work 

    // Create an alias to this type just to make the creation below less 
    // redundant in typing out the long type 
    using vec_type = std::vector<std::shared_ptr<T>>;

    // NON-const object (vector)--so it can be changed 
    vec_type vec_;
    // const pointer to NON-const object--so, vec_p_ can NOT be re-assigned to
    // point to a new vector, because it is `const`! But, **what it points to**
    // CAN be changed because it is NOT const!
    vec_type * const vec_p_ = &vec_;

    // This also does NOT work (in place of the line above) because it makes 
    // the **contents of what you're pointing to const**, which means again 
    // that the contents of the vector can NOT be modified. 
    // const vec_type * const vec_p_ = &vec_; // does NOT work 
    // Here's the compile-time error in gcc when compiling for C++17:
    //      main.cpp: In instantiation of ‘void Container<T>::append(std::shared_ptr<_Tp>) [with T = std::basic_string<char>]’:
    //      <span class="error_line" onclick="ide.gotoLine('main.cpp',78)">main.cpp:78:53</span>:   required from here
    //      main.cpp:61:9: error: passing ‘const vec_type {aka const std::vector >, std::allocator > > >}’ as ‘this’ argument discards qualifiers [-fpermissive]
    //               vec_p_->push_back(std::move(item));
    //               ^~~~~~
    //      In file included from /usr/include/c++/7/vector:64:0,
    //                       from main.cpp:22:
    //      /usr/include/c++/7/bits/stl_vector.h:953:7: note:   in call to ‘void std::vector<_Tp, _Alloc>::push_back(std::vector<_Tp, _Alloc>::value_type&&) [with _Tp = std::shared_ptr >; _Alloc = std::allocator > >; std::vector<_Tp, _Alloc>::value_type = std::shared_ptr >]’
    //             push_back(value_type&& __x)
    //             ^~~~~~~~~


    // To prove that vec_p_ can NOT be re-assigned to point to a new vector, 
    // watch this:
    vec_type vec2_;
    // vec_p_ = &vec2_; // COMPILE-TIME ERROR! Here is the error:
    //      main.cpp:44:5: error: ‘vec_p_’ does not name a type; did you mean ‘vec_type’?
    //           vec_p_ = &vec2_; // COMPILE-TIME ERROR!
    //           ^~~~~~
    //           vec_type

    // BUT, this works just fine:
    vec_type * vec_p2_ = &vec2_; // non-const pointer to non-const data 

public:
    void append(std::shared_ptr<T> item)
    {
        vec_p_->push_back(std::move(item));
    }

    void printElements()
    {
        for (int i = 0; i < vec_p_->size(); i++) 
        { 
            // Notice we have to use a double de-reference here now!
            std::cout << *(*vec_p_)[i] << std::endl; 
        }
    }
};

int main(int argc, char** argv) 
{
    std::unique_ptr<Container<std::string>> c = std::make_unique<Container<std::string>>();

    c->append(std::make_shared<std::string>("hello"));
    c->append(std::make_shared<std::string>("world"));

    c->printElements();

    return 0;
}

Output:

hello
world
Gabriel Staples
  • 36,492
  • 15
  • 194
  • 265