rayimag
rayimag

Reputation: 893

Placement of a method in a Class

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?

Upvotes: 0

Views: 203

Answers (6)

Frerich Raabe
Frerich Raabe

Reputation: 94409

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;
}

Upvotes: 3

Steve Jessop
Steve Jessop

Reputation: 279315

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.

Upvotes: 4

CB Bailey
CB Bailey

Reputation: 792497

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.

Upvotes: 4

The Sheek Geek
The Sheek Geek

Reputation: 4216

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).

Upvotes: 0

RichieHindle
RichieHindle

Reputation: 281685

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.

Upvotes: 0

lhahne
lhahne

Reputation: 6029

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

Upvotes: 0

Related Questions