0

I am trying to replace raw pointers with smart pointers.

class MyObj {
public:
    MyObj() {
        rawContainer = new BigObj();
    }       
    
    const BigObj* GetRawObj() {
        return rawContainer;
    }

private:
    BigObj* rawContainer;
};

When I call auto rawObj = myObj.GetRawObj() I avoid copying BigObj and can only call the functions marked const, so I can't modify it's content.

class MyObj {
public:
    MyObj() {
        std::unique_ptr<BigObj> ptr(new BigObj());
        container = std::move(ptr);
    }

    const std::unique_ptr<BigObj>& GetObj() {
        return container;
    }

private:
    std::unique_ptr<BigObj> container;
};

This time, with auto smartObj = myObj.GetObj(); I can access non-const methods of smartObj.

I understand the definition const std::unique_ptr<BigObj>& GetObj() is making the reference constant, and not the underlying object, but changing it to const std::unique_ptr<const BigObj>& GetObj() creates a compiler error.

Is there a sensible way to do this? I've seen suggestion of just returning a raw pointer from the smart pointer and using it like in the first example. The actual object in MyObj can't be const since it can actually be modified by some methods.

MikeS159
  • 1,884
  • 3
  • 29
  • 54
  • 3
    I think you have the XY-problem. Why do you need the caller to be aware of the fact, that the object (which they cannot modify or destruct) is held by a `unique_ptr<>`? Why not return const ref to the object itself? – lorro Sep 08 '22 at 11:20
  • 2
    You can still return `const BigObj*` from `GetRawObj()` by returning `container.get()`. – wohlstad Sep 08 '22 at 11:20
  • hm, is your `rawContainer` supposed to live longer than your `myObj`? Because otherwise you're leaking memory, and you shouldn't be `new`-allocating this at all. Your current use case doesn't look to me like one for pointers, but for `const MyObj&` (so, const reference) – Marcus Müller Sep 08 '22 at 11:21
  • 2
    My suggestion would be to return `const BigObj&` so that you don't mix `std::unique_ptr` and `std::unique_ptr` (which are different types), and also, unique_ptr is used when you want to convey "there is only a single istance of this pointer!!!", which is by your design broken train-of-thought – stribor14 Sep 08 '22 at 11:23
  • [`std::experimental::propagate_const`](https://en.cppreference.com/w/cpp/experimental/propagate_const) might help. – Jarod42 Sep 08 '22 at 11:23
  • 4
    `const BigObj* GetRawObj() { return rawContainer.get(); }` would be fine BTW. – Jarod42 Sep 08 '22 at 11:24
  • 2
    Don't return a unique pointer reference, return a const raw pointer. – Galik Sep 08 '22 at 11:25
  • By the way: You could assign an object directly to the unique pointer: `container.reset(new object)` – however within the constructor that's not the right place for anyway; you should get used to implement the constructors initialiser list (not to be confused with `std::initializer_list`): `MyObj() : container(new BigObj()) { }` – this prefers direct initialisation by value over default initialisation + assignment; note, too, that some types (references, const members, non-default constructable types, ...) *only* can be initialised this way! – Aconcagua Sep 08 '22 at 11:25
  • '*Don't return a unique pointer reference, return a const raw pointer.'* – or, *provided* the internal object is *always* alive as long as the containing object is (i.e. if you won't ever return a null pointer anyway), preferably a reference to const. – Aconcagua Sep 08 '22 at 11:27
  • 3
    *"I am trying to replace raw pointers with smart pointers."* - Smart pointers are not meant to be used everywhere and passed around the system. They are only ment to be used where the object needs to be automatically deleted. Nowhere else. Only pass a smart pointer if you want to transfer ownership. Otherwise use references or raw pointers. – Galik Sep 08 '22 at 11:28
  • Ok cool. I did think returning a const to the underlying pointer would be the way to go. – MikeS159 Sep 08 '22 at 11:31
  • Might be of help (if not duplicate!): https://stackoverflow.com/questions/6675651/when-should-i-use-raw-pointers-over-smart-pointers – Aconcagua Sep 08 '22 at 11:32
  • @MikeS159 Any particular reason for returning a pointer rather than a reference? Usually the only reasons for using a pointer is if you want to explicitly allow null pointers or if you want to allow the user to modify the value. Neither seems relevant here. Absent these reasons, using a pointer instead of a reference is potentially misleading and error-prone. – Konrad Rudolph Sep 08 '22 at 11:32
  • @MarcusMüller it's a very cut down example. In the real code the destructor will delete it and there are a bunch of places that would make having it a reference less efficient than a pointer. – MikeS159 Sep 08 '22 at 11:32
  • Not always agreeing with Herb, but this one is great: https://herbsutter.com/2013/06/05/gotw-91-solution-smart-pointer-parameters/ – Aconcagua Sep 08 '22 at 11:33
  • @KonradRudolph Err... Allowing modifications is not a criterion for a pointer, (non-) constness is both for pointers and references... – Aconcagua Sep 08 '22 at 11:34
  • 2
    @MikeS159 References are never less efficient than pointers – under the hoods (i. e. when compiled) they are identical to pointers most times anyway, in some cases they don't even exist once compiled... Not technically, but semantically pointers actually are less efficient as you typically need to assume they can contain a null pointer, thus you would need to add a null pointer check – which can be omitted for references. – Aconcagua Sep 08 '22 at 11:37
  • @Aconcagua I meant modification *of the pointer itself* (not of the pointee), since a reference cannot be rebound but a pointer can. It's a niche use-case, for sure. – Konrad Rudolph Sep 08 '22 at 11:39
  • @KonradRudolph Well, don't see much of an issue there, if need be, can be prevented, admitted, needing a bit of extra work (`auto const p = get(); auto& r = *get();`) – actually returning references are more critical in respect of bad usage (`auto r = get();` <- ampersand forgotten; accidentally creating a copy). The big advantage of references is of other nature: They semantically guarantee an object existing, thus not needing to check for null pointers... – Aconcagua Sep 08 '22 at 11:48
  • @Aconcagua you are right! I don't know why I said that. They are pointers because they can be null. – MikeS159 Sep 08 '22 at 12:34

2 Answers2

0

It is perfectly valid to declare const std::unique_ptr<const BigObj>& GetObj(). The problem must be somewhere else.

#include <memory>

class BigObj
{

};

class MyObj 
{
public:
    MyObj(): container(std::make_unique<BigObj>()) {}

    auto GetObj() -> const std::unique_ptr<const BigObj>&
    {
        return reinterpret_cast<const std::unique_ptr<const BigObj>&>(container);
    }

private:
    std::unique_ptr<BigObj> container;
};

auto main()->int
{
    MyObj obj;
    obj.GetObj();
}

https://onlinegdb.com/xzHzI4GgM

Adrian Maire
  • 14,354
  • 9
  • 45
  • 85
0

Like the commenters above is suggesting. Just return a raw pointer. There is no reason to return a reference to the unique_ptr below.

#include <memory>

struct BigObj {
    int bigData = 1000;
};

class MyObj {
public:
    const BigObj& getObj() const {
        return *container;
    }

    // Alternative if you want a pointer instead of a reference
    // (you probably don't if you know that the pointer is non null)
    //const BigObj* getObj2() const {
    //    return container.get();
    //}

private:
    std::unique_ptr<BigObj> container = std::make_unique<BigObj>();
};

int main() {
     auto myObj = MyObj{};

     auto &ref = myObj.getObj();
}

Note: There is syntax if you want to be able to move a ptr out of the object, but I guess that that is not a part of the question.

Lasersköld
  • 2,028
  • 14
  • 20