1

I read multiple times that when a member function only uses API of class it is member of, then it should be made free standing and put in to same namespace, for example, bad practice:

namespace A
{
    class Foo
    {        
         public:
             void printA() const { std::cout << a << std::endl;}
             void printB() const { std::cout << b << std::endl;}
             void printAandB() const 
             {
                 printA();
                 printB();
             }
         private:
             constexpr int a = 1;
             constexpr int b = 2;
    };

}

good practice:

namespace A
{
    class Foo
    {        
         public:
             void printA() const { std::cout << a << std::endl;}
             void printB() const { std::cout << b << std::endl;}
         private:
             constexpr int a = 1;
             constexpr int b = 2;
    };
    
    //now free standing.
    inline void printAandB(const Foo & foo)
    {
        foo.printA();
        foo.printB();
    }
}

Now my question is, what if Foo is inhereting from some other class:

namespace A
{
    class Foo final : public InterfaceFoo
    {
    ....
    };
}

, and I am using a base pointer, then printAandB can't be used, I could just make printAandB member again or make another printAandB which accepts a smart pointer to the base class:

    inline void printAandB(std::shared_ptr<InterfaceFoo> foo)
    {
        foo->printA();
        foo->printB();
    }

. But I am not sure, what is the better choice? Maybe there is an alternative.

3
  • 2
    Aside: I wouldn't take a std::shared_ptr in a function that only interacts with the pointed-to object, just take a InterfaceFoo &
    – Caleth
    Mar 24 at 9:12
  • @Caleth, I don't really understand what you mean, can you elaborate please?
    – a a
    Mar 24 at 9:34
  • 2
    printAandB doesn't need to participate in the ownership of the InterfaceFoo, it doesn't store it anywhere. If the caller has a std::shared_ptr<InterfaceFoo>, they can just dereference it into the call
    – Caleth
    Mar 24 at 10:17

1 Answer 1

2

There are no "best" or "bad" practices in software engineering which one can apply blindly, almost any decision is a tradeoff. What does this mean for this case?

Resisting to make a function which is implemented in terms of existing public members of a class a member itself has some advantages:

  • it keeps the private scope of the class smaller, which makes it easier to reason about changes.

  • it will make the compiler tell you when you try to access the non-public API of the class, which can in reverse give you feedback if the classes public API is as complete as it should be.

Hence, when a member function does not need more than the public interface of a class, it is a candidate for a free function. However, this decision will also come with some disadvantages:

  • one has to pass the subject of the function ("foo") explicitly as a parameter into the function, and repeat it all over the place (instead on relying on the implicit "this" parameter of a member function)

  • Since C++ does not have C#-like extension methods, calls of the function look may look different and inconsistent when compared to other member function calls. From the users point of view this may look astonishing, because for them it is be pretty irrelevant how the function internally works.

  • When you apply changes like the ones described in the question (extending the inheritance hierarchy, and moving member functions to the base class), you may have to change the code of the free function accordingly. This involves refactoring the existing printAandB(const Foo & foo) to printAandB(const InterfaceFoo &foo). It may also involve to extend InterfaceFoo by some functions formerly only available in the derived class Foo.

(Side note: the latter does not mean to "make another printAandB", and there is also no need to use smartpointers, as @Caleth mentioned in a comment.)

So if you think a function is a good fit to a class as a member function, because it fits perfectly to the abstraction the class provides (and does not introduce additional dependencies) then make it a member function. Whether it uses only other public member function or not can be seen as an implementation detail in this case and should not be necessarily the primary factor to be taken into account. If you still want to keep the free function outside, then refactor it accordingly, as scetched above.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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