Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

What is the right way to expose resources owned by a class?

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

like image 605
Venemo Avatar asked Apr 16 '14 14:04

Venemo


5 Answers

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.

like image 101
Mike Seymour Avatar answered Nov 04 '22 03:11

Mike Seymour


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.

like image 27
Fulvio Esposito Avatar answered Nov 04 '22 03:11

Fulvio Esposito


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.

like image 26
Andy Avatar answered Nov 04 '22 04:11

Andy


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();
});
like image 26
user3542948 Avatar answered Nov 04 '22 05:11

user3542948


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.

like image 29
Johny Avatar answered Nov 04 '22 04:11

Johny