extract common blocks into functions in C++

There's something I recurently struggle with while working on C++ code.

Let's say I've got a method doing X, Y and then Z. Now I'd like to introduce another method that should do X, Y', Z. If that was plain old C code, I'd then make functions X() and Z() with the common code, declaring them static so that the compiler would now they can be inlined if needed, as no code out of this "module" can call them. The method that's part of the API would开发者_StackOverflow社区 then look like

int M(args) {
   X(foo); // that could e.g. be "check args are valid".
   /* here comes M-specific code */
   Z(bar); // that could e.g. be "update_state"
}

int M2(args) {
   X(foo);
   /* here comes M2-specific code */
   Z(bar);
}

Now, if I do the same in C++, X() and Z() no longer have access to the class' protected/private members. Swapping between .h and .cc file to declare those "helper" X() and Z() as I proceed with code writing somehow tempt me to just copy/paste the common code instead, so I tend to duplicate instead the class, having something that's closer to a (java) interface in .h -- with virtually no member variables -- and then have variables, API methods and "helper" methods all within a class block in the .cc file, that inherits from the "interface".

Yet, I doubt this is good practice with C++, so I'm curious to know what other people do in that case.

---

If X and Z are doing stuff relevant to the class, then make them member functions of the class (and if not, then there's no problem, since their implementations can easily be put elsewhere, out of public view).

If they're not supposed to be part of the public interface of the class, make them private.

If it bothers you that their function signatures show up in the class definition, then there are several ways to restructure your code, in such a way that implementation details aren't exposed.

A common way eg., is to use the Pimpl idiom.

Another way, would be to only expose (abstract) interfaces in the public API, and hide the implementing classes from view. This is not always possible, but when it is, it can be very effective.

---

If I understand you right what you want to achieve is to write the two functions X() and Z() only once for more than one function M(). Like the other comments suggest make them member functions marked as inline.

Additionally to implementing X() and Z() as member functions I would use the Strategy pattern where you have a function M() like this

class ClassTest
{
  private:
    void X();
    void Y();

    Alogrithm* m_algorithm;

  public:
    void M();
    void setAlgorithm( Alogrithm* a ) { m_algorithm = a; }
}

void ClassTest::M()
{
  X();    
  m_algorithm->execute();    
  Z();
}

This eliminates the need for a second function M2(). You only need to have a setter for m_algorithm which is a small object which implements your original function Y(). This way the algorithm can even be changed a runtime.

---

You could move your X and Z functionalities into private member function of your class and mark them with the inline modifier, if you wanted to. This would allow access to private members while making access from outside the class difficult.

---

I think you are wrong from concepts:

• If X() and Z() have common code, this is a design improvement. Refactor them.
• If M1(args) works ok, why are you changing it? Once you refactored X() and Z() you can use them in other method. Just create M2(args)using new X() and Z() plus new features in a new Y()method.

---

You can make the functions into:

1. Make them private members of the class.
2. Or you can put them within an anonymous namespace in the implementatio file.

With option #2 you will not be able to access the private members of the class, while this is a big issue, it can be mitigated by passing in all (and only) the required parameters, and either returning a value or using output parameters (pointers or references).