7

I have a collection of unique_ptrs. Here i want to take some of them and return them to the caller. The caller only needs to read the content, so i wanted to use constant references. But I'm not sure how to do this with unique_ptrs.

Here is some code I used to do this with raw pointers:

class entry
{

};
vector<entry*> master;
const vector<entry*> get_entries()
{
    vector<entry*> entries;
    // peusocode, master contains all entries.
    // only some entries are copied, filtered by some predicate
    copy_if(master.begin(), master.end(), back_inserter(entries), ...);
}

How do I do this with unique_ptrs? I could also use shared_ptr, but the ownership is quite clear and as I mentioned the caller does not need write access.

JFMR
  • 23,265
  • 4
  • 52
  • 76
pschulz
  • 1,406
  • 13
  • 18
  • If you want to return a copy of some of them you can't use `unique_ptr`, because afterwards you will have two pointers to the objects, which breaks the uniqueness. Or do you want to remove some of them from the collection and return them? That would be possible (but not via copy_*). – Alan Stokes Feb 10 '18 at 11:26
  • I was thinking of something like `vector&>` together with `emplace_back`, but that generates `forming pointer to reference type` errors. – pschulz Feb 10 '18 at 11:30
  • 4
    Pass raw `entry const *`; you keep ownership use raw pointers for observers. – Richard Critten Feb 10 '18 at 11:32
  • 2
    You're basically attempting to subvert the lifetime guarantees that `unique_ptr` gives you, so you need to be careful or it will all go horribly wrong. But I would just return `vector` as you have; much simpler. You're not giving the caller ownership so you don't need any sort of fancy pointer. – Alan Stokes Feb 10 '18 at 11:33

5 Answers5

8

The caller only needs to read the content

If the caller won't participate in the ownership of the pointer, then just use raw pointer. In fact, raw pointer can be considered as a pointer without ownership.

std::vector<std::unique_ptr<entry>> master;

std::vector<const entry*>
get_entries()
{
    std::vector<const entry*> entries;

    for ( auto const &ptr : master ) {
        /* if something satisfied */ 
        entries.push_back(ptr.get());
    }
    return entries;
}
llllllllll
  • 16,169
  • 4
  • 31
  • 54
6

Unique pointer is a "value type" containing a pointer.

So you could treat it as if it is a value type.

But it is uncopyable. So, the solution maybe use const references.

This also can't be applied to "vector" types. So, the solution is to use the reference_wrapper

//type alias for readability
using uEntry = std::unique_ptr<Entry>;

std::vector<uEntry> entries;

std::vector<std::reference_wrapper<const uEntry>> getEntries() {

    std::vector<std::reference_wrapper<const uEntry>> priventries;

    std::for_each(entries.begin(), entries.end(), [&](const uEntry &e) {
        if (e->a > 5) {
            priventries.push_back(std::cref<uEntry>(e));
        }
    });

    return priventries;
}

int main(int argc, const char * argv[]) {
    entries.push_back(uEntry(new Entry(5)));
    entries.push_back(uEntry(new Entry(7)));
    std::cout << getEntries().size();
    return 0;
}
TankorSmash
  • 12,186
  • 6
  • 68
  • 106
user9335240
  • 1,739
  • 1
  • 7
  • 14
  • I like this. While it looks a bit more complicated, it also stays at the notion of constant references, which 'feels more c++-ish' than pointers. – pschulz Feb 11 '18 at 11:46
  • Shouldn't you use `std::make_unique(5)` instead of `new Entry(5)`? – ph_0 Nov 06 '19 at 15:11
1

The type of the elements contained by a std::vector must be assignable. References can't be assigned-to (only initialized).


If using raw pointers is not an issue, becasue the caller doesn't take part in the ownership, you could use raw pointers to const entry instead:

vector<const entry *> get_entries();

If you want to use smart pointers, you have to consider that assigning a std::unique_ptr to a std::shared_ptr will transfer the ownership of the pointer. That is, the moved-from std::unique_ptr object becomes empty: it doesn't own a pointer anymore.

Therefore, you may want to consider a complete transition from std::unique_ptr to std::shared_ptr for your collection.

JFMR
  • 23,265
  • 4
  • 52
  • 76
  • Since it is a `get_entries()`'s local object what is being returned, it will be *moved* at worst. – JFMR Feb 10 '18 at 11:44
  • My point is that it's not good idea for the returned object to be const. It limits how the returned object can be used, You cannot move it elsewhere for example. – eerorika Feb 10 '18 at 11:46
1

If you have a vector of unique_ptr and you want to return some of them to the caller you can iterate on the vector and collect raw pointers in another vector and return it. One efficient when you work with smart pointers convention is to use raw pointers for reading, and leave unique_ptr and shared_ptr for memory management

vector<entry*> collect(const vector<unique_ptr<entry>>& data){
    vector<entry*> result;
    for (auto const & it: data){
      if (ShouldBeCollected()){
          result.push_back(it.get())
      }
    }
    return result;    
}

So keep cool and don't delete raw pointers

Gabriel
  • 3,564
  • 1
  • 27
  • 49
1

By circumstances of OP the solution must look like bellow. I leave to OP how he would apply std::copy_if.

#include <memory>
#include <vector>
#include <algorithm>

class entry {
};

std::vector<std::unique_ptr<entry>> master;

std::vector<const entry*> get_entries()
{
  std::vector<const entry*> entries;
  // only some entries are copied, filtered by some predicate
  std::transform(master.begin(), master.end(), std::back_inserter(entries),
      [](const auto& up){return up.get();});
  return entries;
}
273K
  • 29,503
  • 10
  • 41
  • 64