0

A class owns an object of type U. Through a method it exposes this object as a const U& using a getter (not cheaply copyable, modification is not desired).

A client, now, wants to make use of this API. He wishes to use the instance of U as part of a complex object (which has no interest in changing the API object). Therefore, he has at least the following option: Create a class T with const U& as parameter and a private field of type const U&, where the constructor stores the instance of the API. This has the extreme disadvantage of making the instances of the class extremely unflexible (e.g. no management using std::vectors), which is not desired.

Not a long time ago, I found that one also could use a std::reference_wrapper<const U>, to store the const U&, which would not impose those disadvantages on instances of type T.

The question now is, does this behave like it is expected and is it a good practice to do so?

In the following and here, you can find a working code using this strategy and the types described.

#include <iostream>
#include <memory>

class U{
    public: 
    uint value;
};

class T{
private:
    std::reference_wrapper<const U> _u;

public:
    T(const U& u)
        :_u(u) {}

    const U& GetU(){
        return _u;
    }
};

const U& provideValue(U& u){
    return u;
}

int main()
{
    U internalApiValue;
    internalApiValue.value = 5;

    auto apiValue = provideValue(internalApiValue);

    T container(apiValue);

    std::cout << container.GetU().value;
}

I guess if this is not a good idea, the only alternative would be avoiding const, because else I would impose high restrictions on the users of such methods (methods exposing const U& instead of U& or U)?

JFFIGK
  • 632
  • 1
  • 7
  • 24
  • 2
    What's wrong with `const U *`? Also, removing `const` won't help: the problem is that the *reference* itself is not assignable. – Quentin Jan 16 '19 at 13:34
  • 1
    @Quentin `const U *` opens you up to lifetime issues. While you could still have the issue taking by reference, you would really have break the "rules" to do so. – NathanOliver Jan 16 '19 at 13:37
  • 2
    @NathanOliver I'm advocating `const U &` for parameters and `const U *` for storage, under the assumption that the "no raw owning pointer" rule is followed. – Quentin Jan 16 '19 at 13:39
  • @Quentin Ah. That makes sense. – NathanOliver Jan 16 '19 at 13:43
  • Thanks for the answers and also your input Quentin, I actually did not think about that. – JFFIGK Jan 16 '19 at 13:54

2 Answers2

2

One major issue with your interface is that T's only constructor takes a const U&. This means you could pass a temporary to T and be left with a reference_wrapper to a dead object since const& in an object does not extend the lifetime of the temporary.

To solve this you need to add a deleted constructor that stops you from accepting temporaries. Adding

T(const U&&) = delete;

will do that.

NathanOliver
  • 171,901
  • 28
  • 288
  • 402
  • If I have multiple such parameters, do I have to delete all combinations? `const U&&, const T&&` `const U&, const T&&`, …? – JFFIGK Jan 16 '19 at 13:37
  • 2
    @JFFIGK Most likely. You "need" (in the sense that you the compiler to catch mistakes) to have a deleted constructor for every constructor that could take a temporary. If you have `T(const U& u)`, `T(const U& u, const X&x)` and `T(const U& u, const Y& y const Z& z)` then you need to add `T(const U&&) = delete`, `T(const U&&, const X&&) = delete` and `T(const U&&, const Y&&, const Z&&) = delete` – NathanOliver Jan 16 '19 at 13:41
  • What about already expecting a std::reference_wrapper in the constructor? Code: https://onlinegdb.com/rkter3nz4 Why: https://stackoverflow.com/questions/23973439/why-does-stdreference-wrapperconst-t-not-accept-a-temporary – JFFIGK Jan 16 '19 at 13:44
  • 1
    @JFFIGK That would work. `std::reference_wrapper` does this for you so you can save yourself some work. If you do so though you commit to that interface and expose the internals. That could/will make it harder to change latter if you decide to. – NathanOliver Jan 16 '19 at 13:46
1

This should be doing what you want. It's using std::reference_wrapper<T> in the way it was intended to be used (passing around references in a way that makes them copyable and assignable). I don't see anything wrong with it. From cppreference.com:

std::reference_wrapper is a class template that wraps a reference in a copyable, assignable object. It is frequently used as a mechanism to store references inside standard containers (like std::vector) which cannot normally hold references.

The only potential downside I see is that an std::reference_wrapper<T> may be a bit akward to use and unfamiliar to some. A more common solution to your problem would probably be to just store a pointer in your object instead of a reference. For example:

class T {
private:
    const U* _u;

public:
    T(const U& u)
        : _u(&u) {}
    …
};
Michael Kenzel
  • 15,508
  • 2
  • 30
  • 39
  • What ptr should that be? `const T*? Would I not have the same problem there? (smart pointers would be inappropriate, because I don't own the object) – JFFIGK Jan 16 '19 at 13:38
  • 1
    The fundamental problem you have is that references are not objects themselves (they are merely entities that refer to objects). Wrapping them in an object type like `std::reference_wrapper` solves this problem. A `const T*` is also an object type that is copyable and assignable, so it also does not suffer from the same issue as a plain reference… – Michael Kenzel Jan 16 '19 at 13:41