FairRoot icon indicating copy to clipboard operation
FairRoot copied to clipboard

About getter functions

Open YanzhaoW opened this issue 1 year ago • 0 comments

This issue is dedicated to discuss how we should write (or improve) getter functions in FairRoot.

From the discussion with @dennisklein in PR #1254, we found it's very inconvenient to always check the pointer from getter functions to prevent nullptr dereferencing.

Here are my opinions regarding this issue:

  1. Prefer const auto& whenever possible. The biggest problem of returning auto& or a pointer is it breaks one of the most important principles in OOP design: encapsulation. This principle basically means the resource/data inside an object should be hidden for the change but open for the read. If a user wants to make a change, change it through interfaces. And in some rare cases where the user needs to make a lot of changes, then change the data via a setter. In principle, a getter should only be used for reading, not for changing in an OOP class. (Of course, this does not apply to unique_ptr as it's not an OOP class.)

  2. Provide one single getter function for a resource (usually by a class that owns it). We have GetSink from FairRun and we also have that from FairRootManager. RunID is another mess. We can get runID from FairRun, FairRootManager, FairSource, FairEventHeader (maybe there are more). And users can get direct access to all these classes. Which class should I get the runID? Are they all the same?

  3. Prefer value for the member variable. FairRun owns FairRuntimeDb as it deletes the object in the destructor. So it's better to own it as a value not as a pointer. With this, there is no need to check whether it's nullptr or not.

Surely there are still many cases where we have to return a raw pointer and have a check on it. By no means should this way be always prevented. But with a better design, we could keep it to a minimum.

I'm certainly not a C++ or an OOP expert. If someone disagrees with my points or has some better ideas, please comment below. @janmayer @klenze @dennisklein

YanzhaoW avatar Jul 06 '23 22:07 YanzhaoW