None of what’s written here is cast in stone; at least not yet... Once there’s a lot of code it may be another story.

We want it all: Code that is well designed, readable, documented, flexible, extensible, safe, testable, and last but not least, FAST.
On that last bit, here’s my bible (read it 7 times):

http://developer.amd.com/htmlhelp/optimization/wwhelp/wwhimpl/java/html/wwhelp.htm

Along with other comments, we should mention whether a function is optimized for size or for speed: A rough idea of what the typical expected call count per frame would be, as well as expected maximum. Our unit testing setup and check-list should include performance testing, by the way. Not with the whole engine; I mean a console app for testing, that includes a loop call of the expected most commonly called function(s), that iterates 1000 times, say, and measures the time; so that we can fine-tune individual packages, classes or functions.

The problem with comments is that they aren’t automatically synchronized by the compiler. Code is often changed; comments seldom are. And yet you can’t live without them. But you can minimize them...

Minimize them ?? !!

Yes, by:

1. by writing code that is self-explanatory in the first place, and
2. by avoiding writing comments that should be obvious from looking at the code

HORROR:

    i = &*j;
    i++; /* increment i */

The litmus test, and the question to ask is: Is this comment explaining the intention behind the code? If not, don’t write it. If it’s not, yet it’s needed; it means you haven’t named your functions, classes and variables properly.

Add comment compatibility with Doxygen and LintC++

Name classes to tell what they are; functions to tell what they do; NOT the other way around

HORROR:

    class get_file
    {
        int handle();
        //...

Avoid “roundabout naming” (my own term) whereby things that are unordered are given numbers.

HORROR:

   fruit fruit1 = apples[42];
   fruit fruit2 = bananas[A1];
   eat( fruit2 );
   eat( fruit1 );

or whereby the natural order of things is put into an arbitrary and confusing wrapper

HORROR:

   house *phouses[1] = East_development[7];
   house *phouses[2] = North_development[7];
   house *phouses[3] = West_development[7];
   house *phouses[4] = South_development[7];
   go( 3, young_man );

Use the right STL container for the job, but if you ever use an array, and ordering is irrelevant, name the array like

   cat unordered_litter[9];

otherwise the reader can’t tell the poor cats didn’t get silicon chips with serial numbers implanted in their heads.

Oh, yeah; people are always fighting about whether to use underscores or capitalization for separators...

    TwoWordName
    two_word_name

I definitely like underscores better (I hate capitals); but I’d be willing to compromise and consider using both styles, if we’d use the first for types and the second for instance names, for instance.

I use “_t” postfix to denote typedef -defined types... “_f” for functors... Not sure if I’m suggesting it...

Do you have a question? ...XOR... What do you want me to do?

Name functions so that it is clear what they do; and be clear of what they do before naming them. A function is either a command, or it is a query, never both. If it is a command, the function does what it’s told, and nothing more; –i.e.: It doesn’t sweep the floor AND bring back a souvenir. And if it brings a souvenir; then it sweeps no floor. Unless you’re going to name your function,

    souvenir   sweep_floor_AND_bring_back_a_();

Returning success or failure of an operation has fallen out of fashion since exceptions took exception.

If we’re talking about class member functions,

1. queries are const, and return something
2. commands are non-const, and return nothing.

    float get_value() const;  // This is a Query
    void set_value( float );   // This is a Command

Remember:

• If a command doesn’t need to be non-const, then it either should be static, or it shouldn’t be a class member.
• If a query refuses to compile after declaring it const, then, BINGO!: That’s where the bug was...

A “pure” function not only does not change the class of which it is a member, but in fact it changes absolutely nothing in the galaxy. No butterfly effect either. The Deeplayer standard says: Either a function is a command and returns void; or it returns something and is pure.

Functions returning write-through pointers or references to things are to be avoided if possible. Where not possible, make the name of the function clearly communicate the fact, and make sure to include comments as in the following example:

    some_type /*non-const*/ & get_write_access_to_private_parts_such_and_so(); //non-const

There are exceptions to this rule, however; and mainly it comes down to classes that aren’t really classes, in the intent sense, but rather kind of like proxies, such as the pimpl idiom‘s interface class, or handle’...

//header foo.h
//...
#include <memory>
//don't#include anything that you could just include in foo.cpp
 
class foo //this is the interface class; no inline functions
{
    class foo_impl;
    std::auto_ptr< foo_impl > pimpl_;
public:
    foo();
    foo( foo & ); //yep, non-const; unless you use dark_ptr<> ... (see further down)
    foo & operator=( foo & ); //ditto
    virtual ~foo();
    void do_whatever(); //non-const, to reflect non-constness of foo_impl's corresponding function
    int get_garbage() const;
};

//foo.cpp
#include "foo.h"
#include "all_kinds_of_headers_needed_by_foo_impl.h"
//define the implementation class:
class foo_impl //inline all functions here
{
    int m_int;
public:
    foo_impl() : m_int( 0 ) {}
    foo_impl( int n ) : m_int( n ) {}
    foo_impl const & operator=( int n ){ m_int = n; return *this; }
    //~foo(); //compiler-supplied dtor ok; non-virtual! --you won't inherit it; how would you?; this is in a .cpp file
    void do_whatever(){ my_int = int( 777.7 * ::rand() ); }
    int get_garbage() const { return m_int; }
};
 
//now implement the functions of the interface "foo":
foo::foo() : pimpl_() {}
foo::foo( foo & other ) : pimpl_( other.pimpl_ ) {} //transfer of ownership
foo & foo::operator=( foo & other )
{
    pimpl_.reset( other.pimpl_.release() ); //transfer of ownership
    return *this;
}
foo::~foo(){} //auto_ptr<> takes care of deleting pimpl_;
void foo::do_whatever()
{
    pimpl_->do_whatever();
}
int foo::get_garbage() const
{
    return pimpl_->get_garbage();
}

The advantages of using pimpl’s are many

• Helps reduce the cascading accumulation of header inclusions by moving many #include’s to the .cpp file
• Helps avoid circular header inclusion gottchas
• Interface remains binary-compatible when the implementation is changed. Great for lib’s/dll/so’s...
• Makes interfaces more terse and readable by removing private data and functions to the .cpp file.
• All pimpl interfaces in an entire program have the same size (a pointer)!!!

This last point is an enormous advantage in terms of code size and performance. It means that we can place our “objects” (just the interfaces, really) into all manner of containers, and the container template code will compile to the same assembly code for all, and therefore be optimized by the linker to use a single template instantiation!!!

This last bit has a catch, though: You cant’t place std::auto_ptr<>’s in containers...

My solution is dark_ptr<>; a hack of auto_ptr<> that makes the rhs of the assignment operator and copy ctor const &, by making the pointer member mutable. Using dark_ptr<>, the above code becomes...

//header foo.h
//...
#include "dark_ptr.h"
//don't#include anything that you could just include in foo.cpp
 
class foo //this is the interface class; no inline functions
 
{
    class foo_impl;
    std::dark_ptr< foo_impl > pimpl_;
public:
    foo();
    foo( foo const & ); //YEY!!!! :-D :-D :-D
    foo const & operator=( foo const & ); //YEY!!!! :-D :-D :-D
    virtual ~foo();
    void do_whatever(); //non-const, to reflect non-constness of foo_impl's corresponding function
    int get_garbage() const;
};

//foo.cpp
#include "foo.h"
#include "all_kinds_of_headers_needed_by_foo_impl.h"
//define the implementation class:
class foo_impl //inline all functions here
{
    int m_int;
public:
    foo_impl() : m_int( 0 ) {}
    foo_impl( int n ) : m_int( n ) {}
    foo_impl const & operator=( int n ){ m_int = n; return *this; }
    //~foo(); //compiler-supplied dtor ok; non-virtual! --you won't inherit it; how would you?; this is in a .cpp file
    void do_whatever(){ my_int = int( 777.7 * ::rand() ); }
    int get_garbage() const { return m_int; }
};
 
//now implement the functions of the interface "foo":
foo::foo() : pimpl_() {}
foo::foo( foo const & other ) : pimpl_( other.pimpl_ ) {} //transfer of ownership
foo const & foo::operator=( foo const & other )
{
    pimpl_.reset( other.pimpl_.release() ); //transfer of ownership
    return *this;
}
foo::~foo(){} //dark_ptr<> takes care of deleting pimpl_;
void foo::do_whatever()
{
    pimpl_->do_whatever();
}
int foo::get_garbage() const
{
    return pimpl_->get_garbage();
}

Now the copy ctor and assignment operator have a signature the STL finds conforming; and so now the STL allows you to place foo objects into containers all you want. I made a std::map<> of std::map<>’s (a 2-dimensional map), by wrapping the second map type into a dark_ptr<>, for example. The possibilities are endless...

Why not use boost::shared_ptr<>?

The problem with shared_ptr<> is speed and memory footprint. Those babies have all kinds of fancy features and the kitchen sink; and there are cases where they are just the thing the doctor recommended.
For general use, though, I wanted something lightweight and fast, that

1. I could use pervasively,
2. would ensure deallocation, and
3. could be placed in STL containers.

And while dark_ptr<> may be a cheat, a hack, a lie... and probably the most politically incorrect smart-aleck pointer in the galaxy, it WORKS You just have to remember that it uses ownership transfer on copy semantics, as the fact is intentionally hidden from the interface, to fool the STL (not you).

Where’s it right to use shared_ptr<>?

Well, reference counting, I find, is most useful with singletons. It helps with getting rid of them at the earliest possible time; –i.e.: when nobody needs them any more. But reference counting is so easy to implement in the singleton itself, seems to me it would take longer to type the include path to shared_ptr<> in the boost tree, than to code the feature in; and it will run faster.

There are (rarer) cases of non-singleton classes that could use shared ownership and reference counting. In some cases, they might even inherit each other and be used polymorphically. I’d use shared_ptr for those ...

Wrong way, IMO:

    class my_singleton
    {
        static my_singleton *pinstance_;
        my_singleton();
    public
        static my_singleton *instance()
        {
            if( !pinstance_ ) pinstance_ = new my_singleton();
            return pinstance_;
        }
    }

//cpp file
my_singleton * my_singleton::pinstance_ = 0;

What’s wrong with it?

It would work, chances are, but what’s wrong is that the standard doesn’t guarantee that the static initialization to zero you see in the cpp file, will happen before instance() gets called. Suppose you declare static instances of client classes of my_singleton, that call instance() from their ctor’s... Think about it...

Right way:

    class my_singleton
    {
        typedef enum { e_whatsthatpointeragain, e_hello, e_goodby } e_ref_count_mode;
        my_singleton();
    public
        static my_singleton *instance( e_ref_count_mode rcm = e_whatsthatpointeragain )
        {
            size_t ref_count = 0;
            static auto_ptr<my_singleton> pinstance_; //MUCH better 8-)
            my_singleton *p = pinstance_.get(); //performance hack
            switch( rcm ) //this switch is fast [0,1,2], uses table...
            {
            case e_whatsthatpointeragain:
                break; //most common case executes ultrafast
            case e_hello:
                if( !p ) pinstance_ = p = new my_singleton();
                ++ref_count;
                break;
            default:
                if( !--ref_count ) pinstance_.reset(0);
                p = 0;
            }
            return p; //single return point from functions; another must...
        }
    }

Besides making the pointer an auto_ptr<> and adding some crude ref-counting, the feature that’s relevant to order of initialization is that the pointer is now a static member of the instance() function. The benefit of it is that the standard stipulates that static members of functions must be initialized at the time the function is called for the first time. Not before, not after. And this guarantees its initialization to zero at the time instance() is called for the first time, even if this happens during static initialization.

What’s that bit about “single return point from functions”?

Modern cpu’s have a “return cache”, that allows them to return from functions and continue executing code speculatively, before they even have a chance to unwind the stack. Functions with multiple returns, however, cannot take advantage of the return cache, as the return cache is tagged by function address, and is a 1 to 1 table.

Besides, multiple returns are ugly and error-prone. When you’re looking at a function wondering what it returns, your eyes naturally go down, towards the closing brace; and may miss half the story.

EDIT: Someone at boost seems to have discovered the same truth about singletons... (Notice the absence of a static pointer as class member)... http://boost.org/libs/pool/doc/implementation/singleton.html

“Package”?

Yep. Package.

What I forgot to mention there is, the project panel in MSVC has folders also. They are not real folders, but, whatever they are, they should mirror the tree of real folders.

I also neglected to mention: Each package should also have an associated console program that tests its functionality. Whenever changes are made to a package, it should be verified that it passes the tests. Whenever a bug or deficiency is identified in a package that somehow managed to pass the test, the test should be updated to detect the problem.

I’m talking about testing with a few normal input values and a few out of range input values, and various combos to hopefully get good coverage. Throw performance testing for anything that will execute once per frame or more. Just calling a function 10,000 times and measuring the time it took, is what I mean.

...by deriving a class that inherits the template instance privately, then re-exposes only the necessary functions and operators... Do so always prior to using a template instance... To shorten decorated names... To specialize and improve naming of functions, to better express the intent...

The Visitor Design Pattern has been called “the most poorly named Design Pattern”.

Visitor encapsulates a polymorphic behavior for a hierarchy of classes, without having to change the classes in the hierarchy.

Have you ever found yourself in the situation where you need to code something like...

void Surprise(B* pb, <perhaps other parameters> )
{
   switch( pb->getTypeId() )
   {
   case type_id_1:
      // cast b to type_id_1 and use it
      break;
   case type_id_2:
      // cast b to type_id_2 and use it
      break;
   ...
   }
}

? What to do...?

Standard answer would be to add a pure virtual function to B and implement it differently in all derived classes and use that. That’s Polymorphism’s ABC’s, right? But the problem is,

• Suppose we have new Surprise functions coming up all the time, during the design, and the number of B-derived classes is large...
• Suppose the Surprise function doesn’t need access to B’s private parts... Kind of inelegant to make Surprise a B member function, isn’t it?

That’s when the Visitor Pattern comes to the rescue:

class B; //Base
class D1; //one derived class
class D2; //another derived class
class AbstractVisitor; //Abstract base class Visitor
class MeetInSpace; //some concrete visitor
class B
{
public:
   virtual void please_accept( AbstractVisitor* v ) = 0;
};
class D1 : public B
{
public:
   virtual void please_accept( AbstractVisitor* v ){ v->visit_me( this ); }
};
class D2 : public B
{
public:
   virtual void please_accept( AbstractVisitor* v ){ v->visit_me( this ); }
};

Instead of a forever growing list of Surprise1(), Surprise2() ... SurpriseN() virtual member functions, we only need to have one virtual void please_accept( AbstractVisitor *v ).

Same function, again and again?!

They look the same, but they are different... When D2 says “visit_me( this )” it sounds different than when D1 says it; –the type of this is not the same...

Now,

class AbstractVisitor
{
public:
   virtual void visit_me( D1* ) = 0;
   virtual void visit_me( D2* ) = 0;
};

One pure virtual function for each? Why not visit_me( B* )?

Because each new concrete visitor will have to override visit_me for each B-derived type. We need concrete types as parameters, precisely because Visitor turns the Polymorphism paradigm into a function overload table. You’ll see...

Instead of having the polymorphic behaviors spread all over the hierarchy, we have now a table of overloads for each kind of concrete visitor:

class MeetInSpace : public AbstractVisitor
{
   <parameters> 
public:
   MeetInSpace( <parameters> );
   virtual void visit_me( D1* );
   virtual void visit_me( D2* );
};
//then, in MeetInSpace.cpp
void MeetInSpace::visit_me( D1 *d1 ){ ::start_a_fight( d1, <parameters> ); }
void MeetInSpace::visit_me( D2 *d2 ){ ::do_some_trading( d2, <parameters> ); }
//etc.

Now that you have a visitor with its table of overloaded visit_me’s, you can rewrite the original Surprise() function like,

void SurpriseEncounter( B* b, <perhaps other parameters> )
{
   MeetInSpace v( <parameters> );
   b.please_accept( v );
}

... whenever it makes sense ...

... don’t forget hash_map ...

Today’s programming languages can express the what, but not the why. DesignByContract may be be one step in the why-direction. –FalkBruegmann

... use it! ...

... find link to Scott Meyers’ controversial article ...

Discuss any subjects pertaining to coding standards at the coding standards forum, here:

http://deeplayer.com/phpBB2/viewforum.php?f=16