Question

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:

  • Exposing a const std::unique_ptr<Field>& reference - this feels quite ugly
  • Exposing a plain Field* pointer - this doesn't feel right because the user might be unsure whether he should delete the instance or not
  • Using std::shared_ptr instead - this feels bad because the ownership is not really shared

For 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.)

Was it helpful?

Solution 2

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.

OTHER TIPS

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.

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.

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();
});

If possible, I would try to avoid exposing the internals of Document at all but failing that I would either return a const Field& or if you need it to be nullable a const Field*. Both clearly show that Document is retaining ownership. Also, make the method itself const.

You could have a non const version of the method that returns a Field& or Field* but I would avoid that if you can.

This is not yet considered, I guess: return a proxy object with the same interface.

The holder of the proxy object has the ownership of that proxy-object. The proxy holds whatever reference(weak maybe) to the referenced object. When the object is deleted then you can raise some error. Th eproxy object does not have ownership.

Maybe it was already mentioned. I am not so familiar with C++.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top