Let's say I have a library which has a Document
class. An instance of Document
can own several instances of Field
. Field
has multiple subclasses (for example IntegerField
and StringField
), and even the API user can subclass it and supply subclass instances to Document
(let's say the user is allowed to develop a custom type of data to store in a field).
I'd like to expose the Field
instances owned by Document
through an API in such a way that users can interact with them, but without transferring ownership.
What is the right way to do this?
I thought about:
const std::unique_ptr<Field>&
reference - this feels quite uglyField*
pointer - this doesn't feel right because the user might be unsure whether he should delete the instance or notstd::shared_ptr
instead - this feels bad because the ownership is not really sharedFor example,
class Document {
private:
std::map<std::string, std::unique_ptr<Field> > fields;
// ...
public:
// ...
// How is this done properly?
const std::unique_ptr<Field> &field(const std::string &name) {
return fields[name];
}
}
I'm looking forward to your suggestions.
(I also welcome advice about alternative approaches like what @Fulvio suggested.)
I'd return Bar&
(perhaps with a const
).
The user will need to understand that the reference will be invalidated if the element is removed from the map - but this will be the case whatever you do, due to the single-ownership model.
As others have answered from a technical standpoint, I'd like to point you at a different approach and revise your design. The idea is to try to respect the Law of Demeter and don't grant access to object's sub-components. It's a bit harder to do, and without a specific example I can't provide many details but try to imagine a class Book formed of Pages. If I want to print one, two or more pages of the book, with your current design I can do:
auto range = ...;
for( auto p : book.pages(range) )
{
p->print();
}
while abiding by Demeter's you'll have
auto range = ...;
book.print( /* possibly a range here */ );
this is slanted towards better encapsulation as you don't rely upon internal details of the book class and if its internal structure changes, you don't need to do anything to your client code.
I'd return a std::weak_ptr< Bar >
(if C++11) or boost::weak_ptr
(if not C++11). This makes it explicit that this is heap memory and doesn't risk dangling a reference to non existent memory (like Bar &
does). It also makes ownership explicit.
I generally don't like handing references to internal members. What happens if another thread modifies it? Instead if I cant hand out a copy, I prefer a more functional approach. Something like this.
class Foo {
private:
std::map<std::string, std::unique_ptr<Bar> > bars;
// ...
public:
// ...
template<typename Callable> void withBar(const std::string& name, Callable call) const {
//maybe a lock_guard here?
auto iter = bars.find(name);
if(iter != std::end(bars)) {
call(iter->second.get());
}
}
}
This way the owned internal never "leaves" the class that owns it, and the owner can control invariants. Also can have niceties like making the code a noop if the requested entry doesn't exist. Can use it like,
myfoo.withBar("test", [] (const Bar* bar){
bar->dostuff();
});
I usually return references to the data not the reference to the unique_ptr
:
const Bar &getBar(std::string name) const {
return *bars[name];
}
If you want to be able to return empty item you can return raw pointer (and nullptr
in case of empty). Or even better you can use boost::optional
(std::optional
in C++14).
If there is possibility that the reference would survive longer than the owner (multithreaded environment) I use shared_ptr
internally and return weak_ptr
in access methods.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With