I have some existing code that in many places passes

boost::optional<std::shared_ptr<Data>>

as a parameter to a function.

The function itself does not need "ownership" of the function so copying the shared pointer is both inefficient and misleading. If it wasn't for the 'optional' I would change this to take a Data const& instead and deference the shared pointer in the call.

However I can't do this with optional as you can't have an optional reference.

I've seen alternative of just passing a raw pointer instead of a reference as it exactly fits the semantics required. However objections are sometimes raised to using raw pointers rightly or wrongly.

Is there a better way to pass this value other than just as a Data const * that ideally works in C 11 but any thoughts are required.

Closing and Linking to other answers are welcome if this is a duplicate, but the other answers I've seen tend to just say that optional<T&> is wrong or to use a T* without really talking about best style or alternatives.

Edit: Changed to boost::optional instead of std::optional as that is what is being used as the codebase is for the moment C 11

CodePudding user response：

As you are considering to change the function to take a Data const& argument, I assume the function is doing nothing when the optional is empty. If that is the case you can simply not call the function when there is no value:

#include <optional>
#include <memory>
#include <iostream>

struct foo {};

int bar(const foo& f, int y) {
    std::cout << "hello\n";
    return 42;
}

template <typename F,typename T>
void call_if(std::optional<std::shared_ptr<T>> t,F f){
    if (t) {
        f(**t);
    } else { 
        std::cout << "not called\n";
    }
}


int main() {
    std::optional<std::shared_ptr<foo>> x;
    int res = 0;
    call_if(x,[&res](const foo& f){ res = bar(f,123);});
    x = std::make_shared<foo>();
    call_if(x,[&res](const foo& f){ res = bar(f,123);});
}

Live Demo

If you do want to call the function in any case consider to pass a optional reference. You are right that std::optional<T&> is not correct, but consider (from cppreference)

There are no optional references; a program is ill-formed if it instantiates an optional with a reference type. Alternatively, an optional of a std::reference_wrapper of type T may be used to hold a reference.

Though you still need to explicitly check if the optional does contain a value to perform the conversion:

#include <optional>
#include <memory>
#include <iostream>
#include <functional>

struct foo {};
using opt_t = std::optional<std::shared_ptr<foo>>;
using opt_ref_t = std::optional<std::reference_wrapper<foo>>;

void bar(opt_ref_t t) {
    if (t)  std::cout << "hello\n";
    else std::cout << "empty\n";
}

opt_ref_t get_ref(opt_t t) {
    if (t) return {**t};
    else return {};
}

int main() {
    std::optional<std::shared_ptr<foo>> x;
    bar(get_ref(x));
    x = std::make_shared<foo>();
    bar(get_ref(x));
}

Last but not least, std::optional<std::shared_ptr<Data>> is a rather odd type to begin with. A std::shared_ptr can already be "emtpy", there is no point in wrapping it in a std::optional. I suppose this is due to legacy code and you cannot change it. If you can, I suggest to change it.

Moreoever, there is nothing wrong with passing a raw pointer rather than std::optional<std::reference_wrapper<T>> when nullptr is a valid input for a function:

void bar(foo* f) {
    if (f) { 
        // do something
    } else {
        // do something else
    }
}

CodePudding user response：

First, there is nothing wrong with passing a raw pointer provided:

1. the function does not store the pointer for later use (takes/shares ownership)
2. passing nullptr to the function is meaningful

But lets look deeper into your type:

boost::optional<std::shared_ptr<Data>>

So this can be a shared pointer or none. And a shared pointer can also be a nullptr. So there are 3 ways to call your function:

foo(boost::none);
foo(boost::make_optional(std::shared_ptr()); // nullptr
foo(boost::make_optional(std::make_shared(data)));

Does your function really make a distinction between none and nullptr. That exists but is rather rare.

If not then the optional is unnecessary.

PS: Don't forget to check the std::shared_ptr before use. I really wish there would be a std::shared_ref.