3

I'm trying to design an API for an object manager that registers and retrieves different related objects. When I want to retrieve an object, I can query it by its object id. I'm wondering if I should be returning a smart pointer or a reference in this case. Here's code to fully illustrate the design and some associated scenarios:

// relevant classes portions:
class ViewManager
{
   std::unordered_map<int, std::shared_ptr<cView>> mViews;
   cView & getView(const uint32_t v){ return *mViews[v]; }
   std::shared_ptr<cView> getViewPtr(const uint32_t v){ return mViews[v]; }
}
class cView;

int main()
{
   // usage
   ViewManager &viewman = ViewManager::getManager(context);
   viewman.getView(static_hash("window1")).doFirstThing(arg1, arg2);
   viewman.getView(static_hash("window1")).doSecondThing(something, arg1);
   viewman.getView(static_hash("window1")).doThirdThing(x,y,z);

   // the below function takes a std::shared_ptr<cView>
   someOtherFunctionSetView( viewman.getViewPtr(static_hash("window1"));

}

My first question is, should I have getView() return the reference as it is now, or should it return the smart_ptr? In the reference's case, I feel it's more convenient to call .doFirstThing(), .doSecondThing(), versus writing out -> to do this. And the -> implies it might be null, so we should check first before using it.

But then when I need to pass the smart pointer to a function that requires it, I need to create a different getter, getViewPtr() in this case.

What are some pros and cons of doing only smart pointers, or doing it this way, or are there any other better ways to make usage of the API as clean as possible?

Another related question I have is, is there a better way of cleanly using this API instead of writing getView() for each line when calling those functions? I actually prefer it this way as it is big and bold that I'm manipulating the same variable over and over. Alternatively, I could write a cView &ref = getView() and use that. Although it would be more efficient, when writing blocks of these for multiple views, it breaks up the flow enough that it becomes harder to read I think? I've also seen APIs that just do one getView() and string .doSomething().doSomethingElse().doSomethingElse(), which seems very foreign to me.

Are there any flaws to any of the designs aside from stylistic choice. As for stylistic choice is there an objective perspective of what would make one style more readable or maintainable vs. another style? Thank you in advance.

1
  • 3
    You need to think about ownership. Do you want the ViewManager to own the views? Then return references or pointers. Do you want to share ownership of the views across your classes? Then return shared pointers. Both can be valid cases that work in practice. Just ask yourself: Does it make sense to use a view after it has been removed from the view manager? If the answer is "no", then you dont need the shared pointer. From my experience, shared ownership can make the program flow pretty complicated, so I try to avoid it if possible.
    – pschill
    Sep 6, 2019 at 8:30

1 Answer 1

1

Relevant documentation:


Both methods appear to use the "operator at", which is the square brackets (operator[]), sometimes called the indexing operator or subscript operator. This is different from the ".at()" method.

  • When using operator-at ([]) on the unordered map, a lookup using a non-existent key will automatically insert a new tuple having the new key and a default value. In order to use operator-at ([]), the unordered map must not be const-qualified, as it requires modification just in case.
  • When using the at-method (.at()), a lookup using a non-existent key will throw an exception. The at-method can be used with unordered map whether it is const-qualified or not. This method does not alter the collection of keys owned by this unordered map.

Because both methods appear to use the "operator at" ([]), the following applies:

  • If the retrieval key doesn't already exist in the unordered map, a tuple is automatically inserted into the unordered map, using the retrieval key, and a default-constructed smart pointer having value of null.
  • If the retrieval key exists but the corresponding pointer is null, the behavior is undefined.
  • If the retrieved pointer is valid, the caller can use it.

The differences between these methods:

  • cView& getView(...)
    • It does not extend or protect the lifetime of the object while the caller (user) is using it.
  • std::shared_ptr<cView> getViewPtr(...)
    • It creates a shared ownership of that object; that shared ownership is handed back to the caller; the caller can potentially choose to keep it, even after the calling method finished. For example, the calling method can store the shared pointer somewhere that survives longer than the method's duration.
    • Therefore, it extends and protects the lifetime of the object.

Additional considerations if multithreaded execution can possibly occur in the application:

  • Because the first variant cView& getView(...) does not extend or protect the lifetime of the object, there is a possibility that the object could be destroyed by another thread which had access to the Manager and had the ability to cause the same object to be destroyed, while the caller (user) on the current thread is still using the object. Therefore, the first variant do not guarantee safety in multithreaded applications.
  • However, there are software coding styles in which all objects that are ever added to a Manager will live for at least as long as the Manager itself, only to be destroyed when the Manager's own destructor is invoked. When such software coding styles are applied, one could reason that for as long as the Manager itself is kept alive, that those objects owned by the Manager will also continue to be alive. Thus, the possibility of premature destruction and dangling reference may not occur in practice.

These safety issues tend to require human analysis. Currently it is hard to automate such reasoning with automated source code analysis.


To reduce typing while doing the equivalent of getView(...), you can choose to implement your own subscript operator on ViewManager. You can browse to the subscript operator implementation on unordered_map to see how it is done in the C++ standard library source code.

This is generally permissible, unless there is a possibility of confusion or ambiguity. For example, if ViewManager is responsible for managing two or more collections for different kinds of objects, then it would be inappropriate to implement your own subscript operator on ViewManager.


Fluent interface is what enables method chaining. One criteria for method chaining is that all of these "chainable" methods must be setters, not getters. The return type of these methods must be a reference to the object itself, in this case, cView&. These methods will not be able to return something else.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.