16

I recently switched to C++11 and I'm trying to get used to good practices there. What I end up dealing with very often is something like:

class Owner
{
private:
    vector<unique_ptr<HeavyResource>> _vectorOfHeavyResources;
public:
    virtual const vector<const HeavyResource*>* GetVectorOfResources() const;
};

This requires me to do something like adding a _returnableVector and translating the source vectors to be able to return it later on:

_returnableVector = vector<HeavyResource*>;
for (int i=0; i< _vectorOfHeavyResources.size(); i++)
{
    _returnableVector.push_back(_vectorOfHeavyResources[i].get());
}

Has anyone noticed similar problem? What are your thoughts and solutions? Am I getting the whole ownership idea right here?

UPDATE: Heres another thing: What if one class returns a result of some processing as vector<unique_ptr<HeavyResource>> (it passes the ownership of the results to the caller), and it is supposed to be used for some subsequent processing:

vector<unique_ptr<HeavyResource>> partialResult = _processor1.Process();
// translation
auto result = _processor2.Process(translatedPartialResult); // the argument of process is vector<const HeavyResource*>
Witek
  • 259
  • 1
  • 3
  • 9
  • Please post code that didn't have this problem - before you switched to C++11 (from C++03? With or without `boost`?). I cannot imagine how this problem could appear just by introducing `unique_ptr`. – anatolyg Apr 29 '15 at 08:49
  • FYI If the only reason your are storing smart pointers in your vector is to keep the heavy resource on the heap then you don't need to because a vector stores its contents on the heap already. – Galik Apr 29 '15 at 08:49
  • 3
    Not related to the constness problem (assuming that's important to your question), but if you want to *share* the pointers, then perhaps you don't actually want `unique_ptr`... – eerorika Apr 29 '15 at 08:50
  • anatolyg: without smart pointers I gues I would just cast from vector to vector Galik: returning/pushing back/etc. a vector of solid HeavyResources would copy them which is costly by definition. – Witek Apr 29 '15 at 08:53
  • 4
    I dont want to share the OWNERSHIP, just allow to look into the resources. – Witek Apr 29 '15 at 08:54
  • It depends on what your objects actually contain, of course. But you may find the extra cost of accessing each element through a pointer can outweigh the cost of copying. Especially in a `std::vector` that benefits from storing its elements contiguously. When you use pointers the elements are not contiguous and don't work as well with the CPU's cache. – Galik Apr 29 '15 at 08:56
  • @Witek you don't need to share OWNERSHIP with `shared_ptr`. However, returning vector of `weak_ptr` would have the same problem you have now. – eerorika Apr 29 '15 at 08:58
  • @Witek I agree with you about the ownership and I see nothing wrong with returning raw pointers. However, once you have written the function to create a new `std::vector` you might as well have written the destructor to delete your pointers and forget about `std::unique_ptr` altogether. Its not the fashion these days but I would be tempted to consider that. – Galik Apr 29 '15 at 09:10
  • Your method of preparing the vector to return is very boilerplate-ish. I'd at least write a helper that could (possibly recursively) traverse such a structure an return a weak one. – Bartek Banachewicz Apr 29 '15 at 10:12
  • @Witek: Would that cast be leagle? If we are considering answers that rely on UB, then you can - in most cases - do the same with `uniqe_ptr` (as long as you don't have a strange custom deleter). – MikeMB Apr 29 '15 at 10:26
  • @MikeMB: http://stackoverflow.com/questions/28574542/is-it-ub-to-cast-away-const-and-read-value answers that i think. – Witek Apr 29 '15 at 10:54
  • @Witek: You have to realize, that just because you can `const/reinterpret_cast` `T`t o `B`, it is not valid to cast `std::vector`to `std::vector`. They are unrelated types. E.g. there could exist completely different specializations of `std::vector` for `B` and `T` and as far as aliasing is concerned, the optimizer is -without further analysis - allowed to assume that any pointer/refernce to the first is is unrelated to any p/r to the second. – MikeMB Apr 29 '15 at 11:11
  • You are right - it's not implied by what I posted. I'm not familiar with language definition, but I could imagine some other rules that would allow us to assume so - like deterministic usage of sizes and alignments while evaluating the templates. (being able to solve it elegantly by the rules of C++03 was never my point though) – Witek Apr 29 '15 at 11:31

4 Answers4

20

I would suggest that instead of maintaining and returning an un-unique_ptred vector, you provide functions to access the elements directly. This encapsulates the storage of your resources; clients don't know that they are stored as unique_ptrs, nor that they are kept in a vector.

One possibility for this is to use boost::indirect_iterator to dereference your unique_ptr automatically:

using ResourceIterator =
     boost::indirect_iterator<std::vector<std::unique_ptr<HeavyResource>>::iterator,
                              const HeavyResource>;
ResourceIterator begin() { return std::begin(_vectorOfHeavyResources); }
ResourceIterator end() { return std::end(_vectorOfHeavyResources); }

Demo

TartanLlama
  • 63,752
  • 13
  • 157
  • 193
  • This requires a class containing the GetVectorOfResources to be though of as a Collection. And it makes sense as I think about it. Heres another thing then: What if one class returns a result of some processing as `vector>` (it passes the ownership of the results to the caller), and it is supposed to be used for some subsequent processing: vector> result1 = _processor1.Process(); // translation auto result2 = _processor2.Process(translatedResult); // the argument of process is vector` – Witek Apr 29 '15 at 09:21
  • 1
    In that case, you could make `decltype(_processor2)::Process` take a range instead and pass indirect iterators to it. – TartanLlama Apr 29 '15 at 09:28
1

If you encounter this often, it might make sense to write a class, that behaves like a unique_ptr, but passes the constness of the pointer to the object it points to. That way, you can just return a const reference to your vector.

I ended up writing this once and be done with it:

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

//unique,const-preserving pointer
template<class T>
class ucp_ptr {
    std::unique_ptr<T> ptr;
public:
    ucp_ptr() = default;
    ucp_ptr(T* ptr) :ptr{ ptr }{};
    ucp_ptr(std::unique_ptr<T>&& other) :ptr(std::move(other)){};

    T&        operator*()       { return ptr.get(); }
    T const & operator*()const  { return ptr.get(); }

    T*        operator->()      { return ptr.get(); }
    T const * operator->()const { return ptr.get(); }
};

struct Foo {
    int a = 0;
};

int main() {
    std::vector<ucp_ptr<Foo>> v;
    v.emplace_back(new Foo());
    v.emplace_back(std::make_unique<Foo>());    

    v[0]->a = 1;
    v[1]->a = 2;

    const std::vector<ucp_ptr<Foo>>& cv = v;

    std::cout << cv[0]->a << std::endl; //<-read access OK
    //cv[1]->a = 10; //<-compiler error
}

Of course, you can extend it a bit, if you need custom deleters or want to add a specialization for managing arrays, but this is the base version. I also belive I've seen a more refined version of this somwhere here on SO, but I can't find it right now.

Here is an example, of how this can be used in a class:

class Bar {
    std::vector<ucp_ptr<Foo>> v;
public:
    void add(const Foo& foo){ 
        v.push_back(std::make_unique<Foo>(foo)); 
    }
    //modifying elements
    void doubleElements() {
        for (auto& e : v){
            e->a *= 2;
        }
    }
    const std::vector<ucp_ptr<Foo>>& showElements() const{
        return v;
    }
};

EDIT

As far as your update is concerened, you have to live with the fact that vector<T> is unrelated to vector<B> even if it would be valid to cast T to B and vice versa.
You can write adaptors, that give you a different view to the elements (by casting each element when necessary) but - aside from creating a new vector of the proper type - there exists no general meachanism (that I am aware of) to do what you want.

MikeMB
  • 20,029
  • 9
  • 57
  • 102
  • The problem with this one is that once emplaced inside the Owner, even he's not allowed to modify his resources. But it might be useful. – Witek Apr 29 '15 at 10:15
  • @Witek: Maybe I'm missing your point, but why shouldn't he? Can you give an example of what is not possible with this scheme? – MikeMB Apr 29 '15 at 10:19
  • @Witek: Sorry, there was a typo in my solution: the `operator*`of course returns a `T&` not a `T*` – MikeMB Apr 29 '15 at 10:37
  • You are right - I misunderstood your code at first. I'm also quite surprised that const vector allows access to const elements! – Witek Apr 29 '15 at 11:14
  • Now I can't understand why doesn't `unique_ptrs` work like your `ucp_ptrs`?! If `vector` maps its constness to its elements, why doesn't `unique_ptr` map its constness to its value? – Witek Apr 29 '15 at 11:46
  • @Witek: Essentially, because smart pointers are supposed to work like real pointers and there are a lot of instances, where you want a `T const*` or `T* const`, but not a `T const* const`. – MikeMB Apr 29 '15 at 19:08
0

You might want to consider shared_ptr instead, as it probably reflects what you are trying to achieve. You'll probably want to use unique_ptr in smaller scopes, or any time you wish to reflect only one thing using an object.

Tas
  • 7,023
  • 3
  • 36
  • 51
  • 7
    I would repate my comment from above just to encourage discussion about understanding of ownership: "I don't want to share the OWNERSHIP, just allow readonly access into the resources." In my understanding usage of shared_ptr as in makes the ownership distributed. IMO in proper architectures you can clearly point the ownership. – Witek Apr 29 '15 at 09:05
  • I agree that `std::shared_ptr` should be avoided unless you can't control which object needs to delete the resources. – Galik Apr 29 '15 at 09:16
  • It's entirely up to you; however, you were trying to use `unique_ptr` and noted that you were having difficulty, whereas `shared_ptr` does what you need it to do. I would definitely use `shared_ptr` if I had some `ResourceManager` and needed to temporarily lend out resources. As suggested, you _could_ create helper access functions but that's tedious if callers are allowed to do multiple things with the object. `shared_ptr` does what you need: it shares access. – Tas Apr 29 '15 at 09:59
  • 2
    This doesn't actually address the question. The OP wants to return a collection of pointers to `const`. Wether they are raw, unique or shared actually addresses a different issue and are only relevant for how exactly the methods have to be written. – MikeMB Apr 29 '15 at 10:22
-2

I tried this:

public:
   const std::vector<int const *> getResource()
   {
       return reinterpret_cast<std::vector<const int *> &>(resources);
   }

private:
   std::vector<unique_ptr<int>> resources;

It worked.

vincentB
  • 128
  • 1
  • 8
  • I guess it will fail if you try to copy the returned vector. – Witek Apr 29 '15 at 09:45
  • It didn't... so that's also something to look into (time to understand reinterpret_cast ;) ) – Witek Apr 29 '15 at 09:55
  • This is very dangerous, as it assumes that `std::unique_ptr` just holds a single pointer, which is not necessarily the case, especially if you have a stateful custom deleter. – TartanLlama Apr 29 '15 at 09:57
  • Type punning between unrelated non-POD types is not allowed by the standard, so this program is ill-formed. And the only efficiency gain is probably saving some typing. – Arne Vogel Apr 29 '15 at 10:05
  • @Arne Vogel: While I don't want to encourage any form of UB, it has a significant efficiency gain compared to the OPs solution, as one can now return a reference or pointer to the internal vector instead of creating new vector. But again, any reliance on UB is a catastrophy waiting to happen. – MikeMB Apr 29 '15 at 10:44