29

I have a colleague in my company whose opinions I have a great deal of respect for, but I simply cannot understand one of his preferred styles of writing code in C++.

For example, given there is some class A, he'll write global functions of the type:

void foo( A *ptrToA ){}

or:

void bar( const A &refToA ){}

My first instinct upon seeing global functions like that is: "Why aren't these members of A?" He'll insist up and down that this is consistent with recommendations for good practice in C++, because foo and bar can perform all they need to perform by using the public interface of A. For example, he'll argue that this is completely consistent with Scott Meyers Effective C++ recommendations. I find it hard to reconcile this with item 19 in that book which basically says everything should be a member function with a few exceptions (operator<< and operator>> and functions that need dynamic type conversion). Furthermore, while I agree that the functions can do what they need to do with the public interface of A, in my opinion, that's largely the result of people writing classes that have getters and setters for every data member of class A. So with that public interface, A is an over-glorified struct and you certainly can do anything with the public interface. Personally, I don't think that should be exploited, I think it should be discouraged.

Obviously, this is only possible in a language like C++ that is not pure object oriented, so I guess one way of looking at it is that my colleague does not favor a pure object oriented approach to software design. Does anyone know of any literature that supports this position as a best practice? Or does anyone agree with this and can possibly explain it to me in a different way than my colleague has so that I might see the light? Or does everyone agree with my current feeling that this just doesn't make much sense?

Edit: Let me give a better code example.

class Car
{
    Wheel frontLeft;
    Wheel frontRight;
    Wheel rearLeft;
    Wheel rearRight;
    Wheel spareInTrunk;

public:
    void wheelsOnCar( list< Wheel > &wheels )
    {
        wheels.push_back( frontLeft );
        wheels.push_back( frontRight);
        wheels.push_back( rearLeft);
        wheels.push_back( rearRight);
    }
    const Wheel & getSpare(){ return spareInTrunk; }
    void setSpare( const Wheel &newSpare ){ spareInTrunk = newSpare; }
    // There are getters and setters for the other wheels too,
    //but they aren't important for this example
};

Then I'll see a function like this:

void wheelsRelatedToCar( Car *aCar, list< Wheel > &wheels )
{
    aCar->wheelsOnCar( wheels );
    wheels.push_back( aCar->getSpare() );
}

This is a real example with the names of the classes and functions changed of course. Why would one want wheelsRelatedToCar to not be a member function of Car? In this real example, Car and Wheel were in the same library. The global function was defined in a source file in a specific application using that library, so the argument was made that the function was specific to the application. My response was that it was a perfectly legitimate operation on the Car and belonged with the Car class. Is there another perspective to look at it (other than one who does not prefer to use object oriented design)?

Lightness Races in Orbit
  • 378,754
  • 76
  • 643
  • 1,055
Mutmansky
  • 577
  • 6
  • 16
  • 1
    You have shown us a little too less code to form an opinion. But of course, this is my opinion. YMMV. – dirkgently Oct 28 '09 at 16:31
  • 2
    Are these global functions in the global namespace? If this is the case then you will probably run into problems with naming conflicts. Otherwise, the answers provided supporting your colleague are correct, as long as the class acted on follows good encapsulation practices. – kidnamedlox Oct 28 '09 at 17:28
  • To reference the more detailed example, the wheelsRelatedToCar() function was a global function in a file called AppUtilities.C (where App was the actual App name). I guess my point is that who would ever think to look for that function in a "Utilities" file? It is conceptually an operation on the Car and therefore belongs in the Car class. When I add a second spare on the roof of the car which I should be able to do if Car is appropriately encapsulated, this global function in the Utilities class doesn't work as expected any more. Fundamentally breaks encapsulation IMHO. – Mutmansky Oct 28 '09 at 17:39
  • Your example, IMO, is in fact, extremely questionable, but there's no absolute right or wrong. Pedantic adherence to any extreme - too rich a public interface or too skimpy of one with lots of helper globals, is silly. To me, this boils down to name-space clutter: Does that global function really belong in the global namespace? Does it provide any utility by being there? If not, and it is directly associated with the class itself, then make it a member function. I for one would not like to work with software for which there are tons of extrenal funs when they could have been members. – Mordachai Oct 28 '09 at 18:41
  • Of course, we all want to know what they say when you show them this page. :) – jmucchiello Oct 28 '09 at 19:14
  • I probably won't show him. I'm not much for confrontations and I doubt he would be convinced. It would just cause more problems then it would solve. The goal was not to embarrass him or get a bigger stick to beat him with, but more for my own knowledge. And I think the answers did advance my own knowledge even if they didn't change contradict my initial thoughts in this case. – Mutmansky Oct 28 '09 at 19:57

10 Answers10

22

Scott Meyers has advocated that non-member functions often improve encapsulation:

Herb Sutter and Jim Hyslop also talk about this (citing Meyer's article) in "Self-Sufficient Headers"

These ideas have been republished (in more refined form) in the 3rd edition of Meyer's "Effective C++", "Item 23: Prefer non-member non-friend functions to member functions ", and Sutter/Alexandrescu's "C++ Coding Standards", "44 - Prefer writing nonmember nonfriend functions".

I think a lot of developers find this non-intuitive and maybe a little controversial.

Michael Burr
  • 333,147
  • 50
  • 533
  • 760
  • 3
    Scott Meyers does not suggest making lots of getters and setters to improve Encapsulation through non-member functions. Does he? – jmucchiello Oct 28 '09 at 16:33
  • 4
    Getters and setters are, in general, evil, although they're better than public data members. – David Thornley Oct 28 '09 at 16:46
  • Even though an example about setting the components of a Point class is used int he article, it's not about getters/setters - it's just a simple class with an easy to understand interface to use to get the point across (::rimshot::). – Michael Burr Oct 28 '09 at 16:59
  • 4
    Yes, but by showing the example full of getters and setters and then declaring from on high that you "improve" encapsulation through non-member functions, perhaps some people get the wrong message from the article. – jmucchiello Oct 28 '09 at 17:15
  • 6
    I'm not vilifying Meyers for slap-dash choice of example. I saying perhaps in this case he should have taken more care. He understands inherently that a class' public interface should be streamlined and minimal. That is in fact what the article is really about. But he focused on how great non-member functions are to the extent that he forgot that not every C++ programmer knows the first part: streamline the public interface first. There's a place for focus and place for reinforcing the basics. The Questioner's "guru" friend apparently missed the forest for the article's trees. – jmucchiello Oct 28 '09 at 17:32
  • Unless I'm missing something, the Point class is the only one that involves getters/setters. The other examples do not. Even so, it's quite common for tutorial articles to use simple, otherwise not-great techniques in order to keep focus on what's being taught. How many times do you read about example code not performing proper error handling in the interest of not distracting from the intended focus? While this may not be ideal in a purist sense, it's a reasonable thing to do in many cases. – Michael Burr Oct 28 '09 at 17:34
  • 1
    I agree that the article isn't a full exposition of the topic of class design, and I'm sure Meyers would agree with you on that point. He'd probably tell you to read the book for a bit more depth (though I'm pretty sure he'd say the book also isn't really a full study on class-design). However, I think the article gets the point across about the potential usefulness of a design using nonmember functions, and the article is online while the book chapters (for either Meyers or Sutter/Alexandrescu) aren't as far as I know. – Michael Burr Oct 28 '09 at 17:45
  • 1
    I prefer my explanation. You are saying they didn't address design concerns because of space. That's bad. I'm saying they didn't address design concerns because it didn't occur to them that anyone would take their article and use it to make wide open classes to __facilitate__ the non-member function paradigm they were espousing. – jmucchiello Oct 28 '09 at 17:49
  • I think this is the best answer because I think this article is exactly where my friend got this idea embedded in his head. However, I think I agree with the comment above that he also missed the forest for the trees. I think in my more detailed example above, if the class did not have a polluted public interface to begin with, the question would be moot, because he would have had to make his wheelsRelatedToCar() function a member function. – Mutmansky Oct 28 '09 at 18:17
  • 2
    It's usually difficult to account for specifities, many classes are just glorified structs in the end. However I would distinguish such a class from a "blob". The blob consists in putting together elements that have little relation, while it's quite reasonable to group the 5 wheels of a car in the same object. In the end, I have always distinguish 2 kinds of classes: the ones with a clear responsability (handling a resource for example), and those which are in fact aggregates of related objects. – Matthieu M. Oct 28 '09 at 18:36
21

Herb Sutter & Andrei Alexandrescu recommandation about that:

Avoid membership fees: Where possible, prefer making functions nonmember nonfriends.


Non-member nonfriend functions:

  • improve encapsulation by minimizing dependencies: The body of the function cannot come to depend on the nonpublic members of the class.
  • reduce coupling, by breaking apart monolithic classes to liberate separable functionality
  • improve genericity, because it's hard to write templates that don't know whether or not an operation is a member for a given type.

Now, to answer your question (when ?), here is an algorithm to determine whether a function should be a member and/or friend:

If the function is one of the operators =, ->, [], or (), which must be members:
=> Make it a member

Else if: 
    a) the function needs a different type as its left-hand argument (as do stream operators, for example); 
 or b) it needs type conversions on its leftmost argument; 
 or c) it can be implemented using the class's public interface alone:
=> Make it a nonmember (and friend if needed in cases a) and b) )

If it needs to behave virtually:
=> Add a virtual member function to provide the virtual behavior, and implement the nonmember in terms of that

Else: 
=> Make it a member

References:

  • H. Sutter and Andrei Alexandrescu . C++ Coding Standards (Addison-Wesley, 2004)
  • S. Meyers . "How Non-Member Functions Improve Encapsulation" (C/C++ Users Journal, 18(2), February 2000)
  • B.Stroustrup . The C++ Programming Language (Special 3rdEdition) (Addison-Wesley, 2000). §10.3.2, §11.3.2, §11.3.5, §11.5.2, §21.2.3.1
  • H. Sutter . Exceptional C++ (Addison-Wesley, 2000).
  • H. Sutter . Exceptional C++ Style (Addison-Wesley, 2004).
KeatsPeeks
  • 19,126
  • 5
  • 52
  • 83
13

You answer your own question. If the global functions can operate only using the sleek, streamlined, and unbloated public interface to the class, then they should be global. If the class has been morphed to make these functions possible, then it is not so good.

jmucchiello
  • 18,754
  • 7
  • 41
  • 61
13

OK

  1. Your mate is correct, if a method doesn't need to be a member of a class, then - strictly speaking - it should not be a member of a class.
  2. You are correct, exposing every item on a class, so you can write global functions, is wrong.

Your mate is correct because it's easier to build generic methods if they're not part of a class (e.g. Why is std::find not a member of std::vector? because then it wouldn't work for lists, maps etc).

You are also correct, because exposing too much to do this stuff breaks encapsulation.

When you get out into the real world and start writing apps, and working to business deadlines, you will find that every application is a competing set of requirements. "We need to get A, B, & C in, but by the end of next month. That's, impossible, they can have B & C, or A & C, but not A & B. Or they can have a not-ideal version of A & B and a good version of C".

Writing code is no different, there are plenty of laws and rules that define ideal levels of encapsulation, genericity, cohesion etc, but many of them are contradictory, and you spend so much time trying to satisfy them all that you get nothing done.

I've always said that these principals and "laws" are actually just guide lines, follow them where you can, find your own level where they sit well with you . . . and expect those levels to change every 6 months or so :)

Hope this helps.

Binary Worrier
  • 50,774
  • 20
  • 136
  • 184
6

One way to look at this is this:

  • If a function manipulates an object's inner state, then that's a good indication that this function should probably be a member function.
  • If a function uses an object without changing its inner state, then that's a good indication that this function probably should be a free function.

However, this doesn't mean that it is a good idea to exclusively follow these guidelines in all cases. There are other considerations, too. For example, as has been quoted by others, non-member function contrary to popular belief often increase encapsulation. (Of course, if they deal with an object's state by means of getters/setters to private data, then that's more than questionable. In fact, I find getters/setters questionable anyway. See this excellent article on this topic.)

sbi
  • 219,715
  • 46
  • 258
  • 445
4

In addition to already mentioned articles, I think it is worth to quote several additional opinions from experts:


While we could make a member function to return length, it is better to make it a global friend function. If we do that, we will be able eventually to define the same function to work on built-in arrays and achieve greater uniformity of design. I made size into a member function in STL in an attempt to please the standard committee. I knew that begin, end and size should be global functions but was not willing to risk another fight with the committee. In general, there were many compromises of which I am ashamed. It would have been harder to succeed without making them, but I still get a metallic taste in my mouth when I encounter all the things that I did wrong while knowing full how to do them right. Success, after all, is much overrated. I will be pointing to the incorrect designs in STL here and there: some were done because of political considerations, but many were mistakes caused by my inability to discern general principles.)


  • Bjarne Stroustrup. The C++ Programming Language:

10.3.2 Helper Functions

Typically, a class has a number of functions associated with it that need not be defined in the class itself because they don’t need direct access to the representation. For example:

int diff(Date a,Date b); // number of days in the range [a,b) or [b,a)
bool leapyear(int y);
Date next_weekday(Date d);
Date next_saturday(Date d);

Defining such functions in the class itself would complicate the class interface and increase the number of functions that would potentially need to be examined when a change to the representation was considered. How are such functions "associated" with class Date? Traditionally, their declarations were simply placed in the same file as the declaration of class Date, and users who needed Dates would make them all available by including the file that defined the interface.


Basically my belief is that nonvirtual member functions in general are an unnecessary cutesy in C++ that wahses people's brains, leads to bad programs, and will take many years to wear off. For smart pointers in particular, they can do even more harm.

Evgeny Panasyuk
  • 9,076
  • 1
  • 33
  • 54
  • 1
    Good points all. I think my struggle with these concepts is that I was coming more from a Java background where everything is a member. I'm training myself to be more open minded and accept the C++ model. I'm growing more used to thinking about stand alone functions as entities (for lack of a better word) unto themselves. I've never favored a very rigid "purity" in any language, but old habits die hard. I also still argue that getters and setters are evil and lead to a polluted public interface. Taking advantage of such pollution is not desirable in my opinion, but it's a fine line. – Mutmansky Nov 11 '12 at 14:08
2

While I agree with most other answers here, that free functions should be used when possible, there's also another side of the coin that needs attention: The pragmatic programmer's view. If I'm new to a code base, the discoverability of behavior for a given type is infinitely higher if that behavior is exposed as member functions. Think intellisense. You will not get that with free functions.

Johann Gerell
  • 24,991
  • 10
  • 72
  • 122
1

So you are saying that a String shouldn't have an "bool IsEmpty() const" public member just because it has "int Size() const" and IsEmpty is defined and implemented as Size() == 0?

I would disagree. For me, having extra features adds more services to the class, and this is generally good if they don't undermine its ease of understanding - which, in the general case, is not necessarily so.

Daniel Daranas
  • 22,454
  • 9
  • 63
  • 116
  • 1
    While I agree with you, your example is perfect for templatizing: template book IsEmpty(const T& t) { return t.size() == 0; } That works on all the STL containers and basic_string. It must be "better". – jmucchiello Oct 28 '09 at 17:23
  • (My apologies) And you can make it work with C strings: template<> bool IsEmpty(const char* s) { return !s || !*s; } I'll leave int and double as an exercise for the more bored than I am. – jmucchiello Oct 28 '09 at 17:27
  • Just for comparison, here's what Herb Sutter thinks of the std::string class: http://www.gotw.ca/gotw/084.htm. On "empty" in particular, he says, "the question really comes down to whether we ought to trade away real benefits in order to follow a tradition, or to change the tradition to get real benefits". In context, he means that making it a member function did the former, whereas the latter would be better if possible. – Steve Jessop Oct 28 '09 at 18:55
  • 1
    @jmucchiello: the only unsatisfactory thing about that template is that it needs to be specialised or overloaded for hypothetical classes which have an O(n) `size()` function, but where you can implement `IsEmpty()` in O(1). Of course you can reasonably say that no such collection should ever exist, but currently the C++ standard says that `empty()` is constant complexity, whereas `size()` merely "should have constant complexity". – Steve Jessop Oct 28 '09 at 19:02
1

Your colleague is right. wheelsRelatedToCar shouldn't be a member function of class Car, because this operation is not even applied (only) to the Car. This function is doing something with the Car and something else with the list of wheels.

So, it should be a member function of some other, higher level class, like class Mechanic or class WheelReplacementOperation. And in case that you don't have any such class (though I think you should have!), better make this function global.

Igor
  • 26,650
  • 27
  • 89
  • 114
  • I still would have to say that I still disagree (maybe I'm just being contrary). If the class had a minimal public interface (i.e. it didn't have the getSpare() and setSpare() methods or any other getters and setters), in order to write the wheelsRelatedToCar method, one would have to add the getSpare() method to the public interface or add the wheelsRelatedToCar() method to the public interface. Personally, my instinct is if I have to add to the public interface, make it a clear, meaningful, method rather than a straight-up getter. Perhaps its just personal style... – Mutmansky Oct 29 '09 at 15:16
  • Furthermore, assuming Wheel and Car are defined in the same "level" (i.e. in the same library for example) I don't think there's any problem with the public interface of Car using a same level or lower level object (Wheel in this case) as a means of communication. – Mutmansky Oct 29 '09 at 15:24
0

Part of the decision to make something a member or non-member also has to do with complexity and managing complexity.

For example, if there are 20 or fewer functions that need to be associated with a class, then why not make them all member functions.

However, when the number runs to 100 or several hundred, it often helps to organize those functions into groups and create a pseudo-class that either stores an instance variable of the other class, or serves as a holder for class/static functions that operate on the other class.

For example, one might have a document object, but an editor class that operates on it and contains the editing functions, rather than having all the editing functions on the document.

There are no hard or fast rules - just principles, such as encapsulate, manage complexity, reduce dependencies. Some of these involve tradeoffs, and have to be analyzed in the context of the problem to which they are being applied.

Larry Watanabe
  • 10,126
  • 9
  • 43
  • 46