Is it good style to use private void member functions in C++?

Question

I am currently writing a class that performs a rather complex computation that should be triggered by one function call.

The steps of using the class would be:

1. Construction including the initialization of the basic parameters
2. Initialization with some more case specific settings
3. The actual computation
4. Now, the user of the class can fetch the outputs that are stored as class variables.

Step 2 is separated from step 1 so you can perform multiple computations with the same parameters by simply changing the case specific ones instead of creating a new object.

The large computation shall now be separated into several auxiliary functions. My approach for this is using private void functions that only handle class variables, e.g.,

class MyComputation {
 public:
  MyComputation(/* some basic parameters */);
  void initSpecificCase(/* some case specific parameters */);
  void doTheActualComputation();
  Results getResults();

 private:
  void computeThisAndThat();
  void computeSomeInterimResults();

  // Inputs, outputs, interim results
};

where doTheActualComputation calls computeThisAndThat and computeSomeInterimResults.

Is this use of private functions good practice? Or should I rather put all the auxiliary stuff into an anonymous namespace? This would result in passing lots of arguments to the functions in the anonymous namespace, also some complex container stuff.

Please note that this question is not about passing arguments but rather about the use of void private member functions with the assumption that the mentioned interim results make sense as class variables as they actually describe the state of the class object.

Answer 1

Decomposing your code into private member functions is entirely OK. By itself, a void return type is not a problem: the function may have an external effect, or may change the state of the class.

But this changing state may be a problem. Your object stores both general state that is used across multiple computations (and likely should not change) and also temporary per-problem state. What happens then the computation throws an exception? It is often difficult to make sure the object is always in a valid state that can perform the next computation when it is reused like this.

It is often a better design to separate these different kinds of data. The per-computation interim results can be stored in an object that is not externally visible and can be thrown away afterwards. This makes managing the life cycle much simpler.

In the header, we define a class that stores the basic parameters and can perform the computation:

/** Usage:
 *      MyComputation comp(basic_params...);
 *      ...
 *      Results r = comp(specific_params...);
 */
class MyComputation {
public:
  MyComputation(BasicParams...);
  ~MyComputation();

  // Could also be a named method, doesn't matter.
  Results operator()(SpecificParams...) const;

private:
  BasicParams... m_basic_params;
};

In the actual source code, we can now define the computation, and importantly: data structures for the per-computation params. We can use static functions or use an anonymous namespace to limit their visibility (technically: to ensure internal linkage):

#include "MyComputation.h"

MyComputation::MyComputation(BasicParams...) ... { ... }
MyComputation::~MyComputation() { ... }

namespace {

struct Params {
  BasicParams const& basic;  // borrow from MyComputation
  SpecificParams params;
  InterimResults results;
  ...
};

void doSomethingInteresting(Params& p) { ... }
InterimResults canReturnOrMutateWhateverYouWant(SpecificParams& p) { ... }
Results extractResults(Params p) { ... }

}  // end anonymous namespace

Results MyComputation::operator()(SpecificParams specific_params) const
{
   Params params { m_basic_params, specific_params, {} };
   doSomethingInteresting(params);
   params.results = canReturnOrMutateWhateverYouWant(params.params);
   return extractResults(std::move(params));
}

Inside this file you are free to use member or non-member functions as is convenient. Non-member functions are workable here because most parameters are already bundled into a single object.

You do not have to leak your internal structure into the header (except of course if this is templated code). You can define whatever data structures you need. Since everything here is effectively private, you do not have to explicitly specify private visibility.