std::optional<T>
contains an object of type T
by value. call_option(option)
invokes a constructor intended for converting two optional
types. This in turn calls a constructor Callable(const Square&)
which, since Square
inherits from Callable
, becomes the implicitly defined copy constructor Callable(const Callable&)
.
As has been pointed out by others in comments, this is called object slicing. The new object is a pure Callable
object, initialized from the Callable
"part" of a Square
object. In this case it is somewhat benign but it can have much worse effects because it copies (or worse, moves) part of an object while ignoring others.
Preventing object slicing
To prevent this from accidentally happening in the first place, there are two rather simple solutions:
- Make base classes abstract
struct Callable
{
virtual int call(int x) const
{
printf("default Callable::call()\n");
return x;
}
virtual ~Callable() = 0;
};
Callable::~Callable() = default;
This prevents compilation with an error like "cannot declare field std::_Optional_payload_base<[…]>
to be of abstract type Callable
". To be clear, any virtual abstract method would work. Adding a virtual destructor has the added bonus that now it is safe to delete Square
objects through pointers to Callable
. In your code something like std::unique_ptr<Callable> c = std::make_unique<Square>()
would have called the wrong destructor, now it works.
- Make base class constructors, assignment and destructors protected
struct Callable
{
virtual int call(int x) const
{
printf("default Callable::call()\n");
return x;
}
protected:
Callable() = default;
Callable(const Callable&) = default;
~Callable() = default;
Callable& operator=(const Callable&) = default;
// add move constructors and assignment as needed
};
This likewise prevents the creation of objects of this type and it prevents destroying Squares
through the wrong destructor (unique_ptr<Callable>
doesn`t compile). This pattern is mostly useful if the base class doesn't have a virtual method and you want to avoid the cost of adding one.
Using either of these patterns is a bit of a coding style question. As such it is opinion-based. However, in my opinion all classes that are designed to be inherited from should be made functionally abstract in this pattern to prevent exactly this issue of happening.
Declaring optional function arguments
Back to the point of how void call_option(const std::optional<Callable>& measurable)
should be declared instead.
In my opinion your call_as_pointer
is exactly right. Any form of smart pointer such as unique_ptr
is pointless, slow and restrictive if all you do is "optionally borrow" a reference. Borrowing objects is one of the last remaining uses of raw pointers.
Even if you hold your Square
objects in unique_ptrs
, you should probably pass them as raw pointers in instances like this here to 1) avoid the extra level of indirection, 2) make it clear that the interest is the object itself, not the pointer to it 3) avoid restricting yourself to only unique_ptrs
in case you want stack-allocated or shared_ptr
objects on some call sites.
One might think that this is a good alternative: call_option(std::optional<const Callable*> measurable)
. However, there is nothing that prevents the measurable
from "having a value" and that value being nullptr
. In fact, call_option(nullptr)
would probably not do what you want.
Another school of thought is that nullptr
values and/or explicit checks for object existence should be avoided. Instead, a do-nothing Callable
may be defined.