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.

Answer 1

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.

Answer 2

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.