tags:

views:

113

answers:

6

I have a C++ class in which many of its the member functions have a common set of operations. Putting these common operations in a separate function is important for avoiding redundancy, but where should i place this function ideally? Making it a member function of the class is not a good idea since it makes no sense being a member function of the class and putting it as a lone function in a header file also doesn't seem to be a nice option. Any suggestion regarding this rather design question?

+1  A: 

You should put it as a private member function. This is what they are for.

lhahne
This is a bad rule of thumb as it can make you end up with lots and lots of private member functions (so you have a large interface exposed to e.g. friends).
Frerich Raabe
Yes. That's true in some cases.
lhahne
A: 

I'm not sure why making a private member function isn't the right answer, but if it isn't, make it a static stand-alone function in the same .cpp file as your member functions.

RichieHindle
Making it a private is inferior in C++, because it then becomes part of the visible (if not accessible, i.e. to `friend`s) interface of the class. Also, an unnamed namespace is the way to do static functions nowadays.
sbi
+4  A: 

Make it a free function in an anonymous namespace in the cpp file that defines the functions that use it:

namespace {
    int myHelperFunction(int size, Bar &target) {
        ...
    }
}

int Foo::doTarget(Bar &target) {
    return myHelperFunction(this->size, target);
}

template <typename IT>
int Foo::doTargets(IT first, IT last, int size) {
    size += this->size;
    int total = 0;
    while (first != last) {
        total += myHelperFunction(size, *first);
        ++first;
    }
    return total;
}

or whatever.

This is assuming a simple setup where your member functions are declared in one header file, and defined in one translation unit. If it's more complicated, you could either make it a private static member function of the class, and define it in one of the translation units containing member function definitions (or add a new one), or else just give it its own header since you're decomposing things a long way into files already.

Steve Jessop
A: 

If your C++ class is the only class whose members use this common functionality, I say place the method in the class that uses the functionality. However, If there is a chance that another class will have to do that same work, then you should move it into a helper class or library (something of that nature).

The Sheek Geek
Why put it into classes at all? See Frerich's comment to lhahne's answer.
sbi
+4  A: 

If the "set of operations" can be encapsulated in a function that is not inherently tied to the class in question then it probably should be a free function (perhaps in an appropriate namespace).

If it's somehow tied to the class but doesn't require a class instance it should probably be a static member function, probably a private function if it doesn't form part of the class interface.

Charles Bailey
+2  A: 

If multiple member functions of your class share common code, you have to look at the code being shared to decide where it goes.

If the shared code can use just the public interface of the class (so it doesn't access private member variables, protected functions etc.), you should consider whether other classes might want to use the same code. If no other class can make use of the function, put the shared code into a global (free) function within an anonymous namespace in the .cpp file, like this:

namespace {
  void mySharedCode() {
     // ...
  }
}

void MyClass::f() {
  doThis();
  mySharedCode();
}

void MyClass::g() {
  doThat();
  mySharedCode();
}

If the shared code cannot be implemented using just the public interface (maybe the code uses private member variables or something), make it a private member function of the class. However, if the shared code uses just a few (say, one or two or there) member variables of your class, you could change that so that the shared function gets those values passed as variables. I.e. use

// GOOD: free function in .cpp file used
namespace {
  int multiply( int a, int b ) {
    return a * b;
  }
}
void MyClass::f() {
  return multiply( m_a, m_b ) + 3;
}

instead of

// BAD: private member function used
void MyClass::multiply() {
  return m_a * m_b ;
}
void MyClass::f() {
  return multiply() + 3;
}
Frerich Raabe
This is almost exactly what I would have written, if this hadn't been written already.
sbi