C++ Private Functions: Whether to pass class member variable by function parameter, or not

Question

Here's a question that comes up again and again in C++ class implementation. I'm curious about what people's thoughts are 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;
};

with

void A::publicCall(void)
{
    f();
}

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

Or the alternative:

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

private:
    void f(CMyClass &x);

    CMyClass m_Member1;
};

with

void A::publicCall(void)
{
    f(m_Member1);
}

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

I think I always prefer the second one because then f can then operate on any instance of CMyClass but, that said, I have lots of code where the first is perfectly valid since f will only ever operate on m_Member1 and I'm really breaking it into two functions to make the code more readable.

Yes, this is more of a discussion question than an "answer" question, but I'm more interested in the reasoning. I'll mark as an answer a response that gives good reasoning or a good criterion.

Also, keep in mind that this is just a toy example. The class will be bigger than this in reality and thus organization is important.

In your second case, you could make `f` static, and it would create nearly identical code to the first case. — Kerrek SB, Mar 22 '12 at 19:13
@Kerrek SB, This is true and is a great point. In reality, there's no reason `f()` needs to be even a member of this class then. It could be its own standalone utility function. Suffice it to say for now, that `f()` is not particularly useful except in the context of this class. — Chris A., Mar 22 '12 at 19:15
Then define `f` as a free helper function only visible in the implementation file. — ipc, Mar 22 '12 at 19:19
@KerrekSB - Unless `f()` needs other members of the class `A`. Granted, it could receive all of them as parameters, but that may not be easy. — rodrigo, Mar 22 '12 at 19:20
@aib, because in the first one (the parameterless `f()`) I have to hardcode in the specific member variable, whereas the second one can pass it as a parameter. So, in the second case, I can have `publicCall()` ultimately call `f(m_Member1)` followed by `f(m_Member2)` etc. — Chris A., Mar 22 '12 at 19:33

juanchopanza · Answer 1 · 2012-03-22T20:44:17.993

3

Since you're asking for opinions, if a standalone f(CMyClass&) function makes sense and is implementable, then I would also favour that option. I would opt for the first case if the operation performed by f only makes sense in the context of class A, if CMyClass only makes sense in A's context, or if it depends on other attributes of A. I think one has have to decide depending on the problem.

edited Mar 22 '12 at 20:44

answered Mar 22 '12 at 19:19

juanchopanza

223,364
34
402
480

"I would opt for the second case if the operation performed by f only makes sense in the context of class A." Did you mean to say "the first case"? – Chris A. Mar 22 '12 at 19:45
@ChrisA Yes indeed, I meant the first case! I will fix that. – juanchopanza Mar 22 '12 at 20:43

score 2 · Answer 2 · answered Mar 22 '12 at 19:27

As always, that depends. Each scenario is different.

In the specific example you gave - first I'd rule out your alternative (void A::f(CMyClass &x)) mainly because it 'smells' bad (as Martin Fowler puts it). It's a private function, and unless you need to use it now for other instances, make it use the member. You can always refactor it if the need arises.

Imagine what would happen if f had 2 parameters. 3 parameters. 10. Would it make sense then to send them each time? Wouldn't it be better to have these parameters members?

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

All of this is assuming that f does need other information which A holds, otherwise I'd move it to be a method of CMyClass.

+1, I like the reasoning here. The 2,3,10, parameter thing is particularly relevant to my class as it gets bigger. — Chris A., Mar 22 '12 at 19:29

score 1 · Accepted Answer · answered Mar 22 '12 at 19:19

1

Ask yourself: Does it have any meaning now, or may it have any meaning in the foreseable future, to call f() with an object other than m_Member1?

If the answer is:

No. Do a parameterless f() as m_Member1 is an intrinsic part of A.
Yes. Do the f(CMyClass &). Even if now you only use m_Member1, that is not an intrinsic property of the classes you handle.
Maybe. Well... I'd say go with the parameterless f(). There is always the option to change your mind (the change if fairly trivial, actually).

Also note that a function f() can call another function g(CMyClass &), but not the other way around. So, depending on what f() does, that may limit your options.

answered Mar 22 '12 at 19:19

rodrigo

94,151
12
143
190

+1, but I'm going to challenge a few parts of what you said in the spirit of healthy debate (mostly because I want reasons): If the answer to your question is "no" then why would you prefer `f()`? Also, why do you seem to prefer the parameterless `f()` over the other? I guess your last two sentences give some reasoning here... – Chris A. Mar 22 '12 at 19:26
First, because if `m_Member1` is considered a unique part of `A` (for example the `Head` is part of the `Body`) then `f()` can be interpreted to be an operation on `A`, even if it needs to touch the `m_Member1` varaible (`Body::pickYourNose()` does not need a reference to your `Head` or your `Nose`). Second: because if you then write a `g(CMyClass&)` that needs to call `f()` you'll now that you were wrong! – rodrigo Mar 22 '12 at 19:31
Also, having two references to the same object in the same place (`member1` and `m_Member1`) without a reason is probably not such a good idea. – rodrigo Mar 22 '12 at 20:22

score 1 · Answer 4 · answered Mar 22 '12 at 19:24

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

Of course, there are cases where you're better off sending the argument every time you call that function, but why would you resend it when you already have on CMyClass object in the A object. If you need both CMyClass objects to interact, you may be better off adding it to CMyClass member function list, rather than to A.

Also, as Clean Code states, you're better off with functions without any arguments, than with ones with arguments. When another programmer tries to read the function, it must decipher/pay attention to the second parameter, besides the function name.

score 1 · Answer 5 · answered Mar 22 '12 at 19:25

I would base the answer on the context. If there are now or might some day be multiple instances of member variables that f might operate on, then sure, pass it/them as parameters. However, if f is operating on specific elements of the state of the instance of A, I would not pass anything to it. There are lots of cases where there will always be only one foo in an A. At that point is becomes silly to make foo a parameter to f. And slightly less efficient, unless f is inline, since not only is the this pointer being passed, but also the address of foo, which is an extra copy onto the stack.

C++ Private Functions: Whether to pass class member variable by function parameter, or not

5 Answers5