The coding standards are the least of your worries!

Posted Jun 1, 2010 16:34 UTC (Tue) by Ed_L. (guest, #24287) [Link]

For the most part I agree, which is probably why Mark suggested limiting or forbidding inheritance in the first GCCC++ Guideline draft. (But I doubt it will fly). Composition vs. Inheritance is a basic question to all OO design, frequently aliased as "ISA vs. HASA". Prsumably, GCC already has some decent OO hacks to guide any OO "newbies" which in this particular context takes on a whole 'nother meaning as I 'spect the hardline GCC C hacks have eschewed OO because they are already aware of most of its pitfalls and simply don't want to deal with them. I expect some lively debate.

And related, all properties that are required to bring an object into a meaningful state should be supplied through the ctor.

Yabut here you run up against the "do you allow exceptions and where?" question,as well as your local definition of "meaningful state." Personally, I use exceptions. Except I try to avoid throwing from a constructor, because that requires a try block around the entire scope of the object. Rather, if an object needs to perform some exceptional operation in order to be initialized (e.g. allocating memory) I usually write a separate throwable Init() method that sets a private is_init flag to let all other methods know whether the object is ready to use. But (a) this isn't always possible, (b) I haven't investigated whether auto_ptr might be used to limit a constructor try block, and (c) YMMV anyway.

The Guide *will* be interesting.

Posted Jun 1, 2010 22:45 UTC (Tue) by Tobu (subscriber, #24111) [Link]

Good guidelines. I especially agree with the ones about keeping variables private not making things mutable lightly.

I'd like to amend the rule about multiple inheritance though; besides an optional base class, it is perfectly fine to inherit from a number of mixins. For example, any class that deals with a non-copyable system resource (such as a socket) should add boost::noncopyable (or the local equivalent) to its parents. And I recall friends can help making data more private to the world at large, and should be allowed to help write things like output operators that are closely related to the class.

Posted Jun 2, 2010 18:50 UTC (Wed) by ikm (subscriber, #493) [Link]

Posted Jun 2, 2010 23:41 UTC (Wed) by daglwn (subscriber, #65432) [Link]

Some guiding principles:

Most of these are good. A few comments and additional suggestions follow.

• Use inheritance only when you are dealing with similar but different behaviors. I.e. only when one inherits to implement pure virtual functions.

This seems too strict to me. There are reasons to inherit beyond overriding virtual functions. The Curiously Recurring Template Pattern is a prime example (replacing dynamic polymorphism with static polymorphism).

• Isolate responsibilities into a class.

I think I know what you mean, but we have to be careful with how we define "class" or "object." A class does not necessarily have all of its functionality in member functions (see below).

• Write down every piece of behavior only once.

If I understand your meaning, this is good in every language. Factoring out common behavior is always important. If I've missed your point, please follow up. It's good to learn from others.

• Variables are private and never public. They may be protected when subclasses need access to them.

I would go even stronger than that. Rather than make the variables protected, make the accessors (possibly getters/setters) protected. That way you minimize impacts of data member changes.

• Avoid using `friend'. Don't use private and protected inheritance, these are meaningless.

This also seems overly restrictive. In general, yes, one doesn't need private or protected inheritance, but they have their uses, especially when composing behaviors from several classes (aha! see the next point :) ).

• Don't use multiple inheritance. It is a can of worms. When you think you need it, you likely have violated the first two principles.

This is by far my biggest pet peeve of most "good C++ practice" lists. MI is incredibly useful. What one usually doesn't want to do is create a diamond hierarchy where base classes have data members. The problem with data members in diamond hierarchies is the ambiguity of initialization. The most-derived class needs to take care of initializing base class data members, which breaks encapsulation. But MI is very, very useful for imparting properties and/or varying behaviors on a common class (template). Mixins are wonderful things. MI gets used all over the place when template metaprogramming is involved (c.f. Modern C++ Design, boost, etc.).

I would add these guidelines:

• Make all virtual functions private with a public API when necessary. This allows one to insert various behaviors (debug messages, etc.) in the public API once and have it apply to all invocations of the (private) virtual function.

Example:

class Base {
private:
  virtual void doItImpl(void) = 0;

public:
  void doIt(void) {
    debug("Calling doIt!");
    doItImpl();
  }
};

class Foo : Base {
private:
  void doItImpl(void) {
    doSomething();
  }
};

• Enforce const correctness (applies to C as well).
• Use RAII liberally.
• Use exceptions.
• Use type generators to specify data structure types. Do not hard-code std::map<>, etc. When C++0x is ready, use template aliases. This allows one to easily swap out data structure implementations and specialize them for various types. This is quite useful when working to optimize runtime.

Example:

template<typename Key, typename Value>
struct MapGenerator {
  typedef std::map<Key, Value> type;
};

template<typename Value>
struct MapGenerator<int, Value> {
  typedef MyFancyEfficientIntMapper<Value> type;
};

typedef MapGenerator<double, std::string>::type DoubleStringMap;
typedef MapGenerator<int, std::string>::type IntStringMap;

• Prefer free functions. This may seem counterintuitive, but actually makes generic programming easier. Compare boost.Range(Ex) and the standard algorithms. Having freestanding begin() and end() functions is super useful. The same pattern applies to many intrinsic operations. Making them free functions instead of members means that one can adapt existing classes to the interface without having to muck around with changing the class (adding member functions, etc.).

And my number one guideline:

• Do not write code unless you have to. This may seem obvious but there are countless examples of projects that refuse to use existing libraries like boost and end up reimplementing them. Badly. Reimplement the stuff if necessary, by all means (to solve proven performance bottlenecks, for example), but make a darn good case for doing so. This is right up there with "Don't optimize prematurely."