C ++ private functions: whether to pass a member variable of a class according to a functional parameter or not

Question

Here is the question that arises again and again in the implementation of the C ++ class. I'm curious what people think here. Which code do you prefer and why?

class A { public: /* Constructors, Destructors, Public interface functions, etc. */ void publicCall(void); private: void f(void); CMyClass m_Member1; }; 

from

 void A::publicCall(void) { f(); } void A::f(void) { // do some stuff populating m_Member1 } 

Or an alternative:

 class A { public: /* Constructors, Destructors, Public interface functions, etc. */ void publicCall(void); private: void f(CMyClass &x); CMyClass m_Member1; }; 

from

 void A::publicCall(void) { f(m_Member1); } void A::f(CMyClass &x) { // do some stuff to populate x, // locally masking the fact that it really m_Member1 } 

I think I always prefer the latter, because then f can work in any instance of CMyClass , but, nevertheless, I have a lot of code where the first is completely true, since f will only work on m_Member1 , and I really break up its two functions to make the code more readable.

Yes, this is more a moot point than a "answer" question, but I'm more interested in reasoning. I will mark as an answer an answer that gives good reasoning or a good criterion.

Also, keep in mind that this is just a toy example. The class will be larger than it really is, and therefore organization is important.

Answer 1

Ask yourself: does it have any meaning now, or may it have any meaning in the future, for calling f() with an object other than m_Member1 ?

If the answer is:

• No. Do f() without parameters, since m_Member1 is an integral part of A
• Yes Make f(CMyClass &) . Even if you only use m_Member1 , this is not an internal property of the classes you are processing.
• Maybe . Well ... I would say go with no parameters f() . There is always the opportunity to change your mind (a change, if it is pretty trivial, in fact).

Also note that the f() function may call another g(CMyClass &) function, but not vice versa. Thus, depending on what f() does, this may limit your options.

Answer 2

Since you are asking for opinions, if the autonomous function f(CMyClass&) makes sense and is realizable, then I would also approve this option. I would choose the first case if the operation performed by f makes sense only in the context of class A, if CMyClass makes sense only in the context of A or depends on other attributes of A I think what needs to be solved depending on the problem.

Answer 3

As always, it depends. Each scenario is different.

In the specific example you pointed out, I first exclude your alternative ( void A::f(CMyClass &x) ), mainly because it "smells bad" (as Martin Fowler says). This is a private function, and if you do not need to use it now for other instances, use it. You can always reorganize it if such a need arises.

Imagine what happens if f has 2 parameters. 3 parameters. 10. Does it make sense then to send them every time? Wouldn't it be better to have these options?

But what if f were to send some of these arguments to other methods of A ? Doesn't it make sense to use members for this?

All this assumes that f needs other information that A has, otherwise I would move it as a CMyClass method.

Answer 4

Is f in the second example better as a static function of A or a global function, or maybe a member function of CMyClass ?

Of course, there are times when you are better off sending an argument every time you call this function, but why do you need to send it again when you already have a CMyClass object in object A. If you need both CMyClass objects to interact, you might be it’s better to add it to the list of CMyClass functions, and not to A

Also, as Clean Code says, you're better off working with functions without any arguments than with arguments. When another programmer tries to read a function, he must decrypt / pay attention to the second parameter, in addition to the name of the function.

Answer 5

I would base an answer to the context. If at some point or at some point there are several instances of member variables that can work, then be sure to pass them / them as parameters. However, if f works with certain state elements of an instance of A, I would not pass anything to it. There are many cases where there will always be only one foo in A. At this point, it is foolish to make foo the parameter f. And a little less efficient, if only f is not built-in, since not only this pointer is passed, but also the address foo, which is an additional copy to the stack.