views:

468

answers:

4

Herb Sutter has said that the most object oriented way to write methods in C++ is using non-member non-friend functions. Should that mean that I should take private methods and turn them into non-member non-friend functions? Any member variables that these methods may need can be passed in as parameters.

Example (before):

class Number {
 public:
  Number( int nNumber ) : m_nNumber( nNumber ) {}
  int CalculateDifference( int nNumber ) { return minus( nNumber ); }
 private:
  int minus( int nNumber ) { return m_nNumber - nNumber; }
  int m_nNumber;
};

Example (after):

int minus( int nLhsNumber, int nRhsNumber ) { return nLhsNumber - nRhsNumber; }
class Number {
 public:
  Number( int nNumber ) : m_nNumber( nNumber ) {}
  int CalculateDifference( int nNumber ) { return minus( m_nNumber, nNumber ); }
 private:
  int m_nNumber;
};

Am I on the right track? Should all private methods be moved to non-member non-friend functions? What should be rules that would tell you otherwise?

+7  A: 

Not all private methods should be moved to non-member non-friend function, but those that does not need access to your private data members should be. You should give access to as little function as you can, to encapsulate your classes as mush a possible.

I strongly recommend reading Effective C++ from Scott Meyers which explain why you should do this and when it is appropriate.

Edit : I would like to add that this is less true for private method then for public ones, although still valid. As encapsulation is proportionnal to the amount of code you would break by modifying your method, having a private member-function , even though it does not requires access to data members. That is because modifying that code would break little code and only code that you have control over.

Gab Royer
+1: The question seemed to make no sense until I read this answer!
RichieHindle
A: 

Hi!

It highly depends on your design and situation you are in. When I write my code I really only put public and protected functions into classes. The rest is implementation detail which is part of the cpp-file.

I often use pImpl idiom as well. Both approaches have in my opininion following benefits:

  • clean interface design
    users dealing with your interface will better understand it without delving into implementation details if they don't have to.
  • decoupling from implementation
    if you change smth in a cpp file without changing the interface you will only have to recompile one cpp-file (compilation unit) and re-link the app => much quicker build times
  • hide dependencies from other classes, which use your interface
    if everything is put into the header file you sometimes have to include other headers, which are "part of your interface" and others will include them no matter if they don't want to. Putting real implementations to compilation units will hide these dependencies.

But sometimes you write a header only libraries or implementations, there you can't put things into compilation units, since this approach would require not only inclusion of your lib, but linking against your lib as well.

Regards,
Ovanes

ovanes
+2  A: 

This question appears to have already been addressed in answering a different question.

Such rules tend to be good servants and bad masters, there are trades-off involved. Is the result more maintainble if you apply the transformation you suggest? Why? I believe that the intended benefit is that by reducing the number of methods directly dealing with the private data of the object you can more easily understand its behaviour and hence make it easier to maintain.

I don't believe that your after example achieves this goal. [You may notice that the "after" example above won't compile anyway, but that's another story.] If we adjust it to implement the external functions purely in terms of public methods rather than internal state (add an accessor for the value) then I guess we've had some gain. Is it enough to warrant the work. My opinion: no. I think that the benefits of the proposed move to external function become much greater when the methods update the data values in the object. I would want to implement a small number of invarient-maintaining mutators and implement the major "business" methods in terms of those - externalising those methods is a way of ensuring that they can only work in terms of mutators.

djna
The question you point to is related but not the same. The original question deals with IDEs and how they help the programmer, not with the problem itself. Then the 'well answered' is quite subjective when you are talking about an answer with _only_ 2 up votes. That answer is not so well considered.
David Rodríguez - dribeas
The question isn't the same but the answer there does address this one. I've adjusted the wording and reduced the subjectivity.
djna
+8  A: 

I believe in free functions and agree with Sutter, but my understanding is in the opposite direction. It is not that you should have your public methods depend on free functions instead of private methods, but rather that you can build a richer interface outside of the class with free functions by using the provided public interface.

That is, you don't push your privates outside of the class, but rather reduce the public interface to the minimum that allows you to build the rest of the functionality with the least possible coupling: only using the public interface.

In your example, what I would move outside of the class is the CalculateDifference method if it can be represented effectively in terms of other operations.

class Number { // small simple interface: accessor to constant data, constructor
public:
  explicit Number( int nNumber ) : m_nNumber( nNumber ) {}
  int value() const { return m_nNumber; }
private:
  int m_nNumber;
};
Number operator+( Number const & lhs, Number const & rhs ) // Add addition to the interface
{
   return Number( lhs.value() + rhs.value() );
}
Number operator-( Number const & lhs, Number const & rhs ) // Add subtraction to the interface
{
   return Number( lhs.value() - rhs.value() );
}

The advantage is that if you decide to redefine your Number internals (there is not that much that you can do with such a simple class), as long as you keep your public interface constant then all other functions will work out of the box. Internal implementation details will not force you to redefine all the other methods.

The hard part (not in the simplistic example above) is determining what is the least interface that you must provide. The article (GotW#84), referenced from a previous question here is a great example. If you read it in detail you will find that you can greatly reduce the number of methods in std::basic_string while maintaining the same functionality and performance. The count would come down from 103 member functions to only 32 members. That means that implementation changes in the class will affect only 32 instead of 103 members, and as the interface is kept the 71 free functions that can implement the rest of the functionality in terms of the 32 members will not have to be changed.

That is the important point: it is more encapsulated as you are limiting the impact of implementation changes on the code.

Moving out of the original question, here is a simple example of how using free functions improve the locality of changes to the class. Assume a complex class with really complex addition operation. You could go for it and implement all operator overrides as member functions, or you can just as easily and effectively implement only some of them internally and provide the rest as free functions:

class ReallyComplex
{
public:
   ReallyComplex& operator+=( ReallyComplex const & rhs );
};
ReallyComplex operator+( ReallyComplex const & lhs, ReallyComplex const & rhs )
{
   ReallyComplex tmp( lhs );
   tmp += rhs;
   return tmp;
}

It can be easily seen that no matter how the original operator+= performs its task, the free operator+ performs its duty correctly. Now, with any and all changes to the class, operator+= will have to be updated, but the external operator+ will be untouched for the rest of its life.

The code above is a common pattern, while usually instead of receiving the lhs operand by constant reference and creating a temporary object inside, it can be changed so that the parameter is itself a value copy, helping the compiler with some optimizations:

ReallyComplex operator+( ReallyComplex lhs, ReallyComplex const & rhs )
{
   lhs += rhs;
   return lhs;
}
David Rodríguez - dribeas
+1 A through and informative study on the topic. Well done!
Tom Leys
Well done indeed!
djna