Peter Schröder
Writing small programs that will be thrown away when you are done with them do not require you to be particularly careful or establish particular styles. Writing large programs that are supposed to last beyond the next homework deadline are a different matter altogether! This note is intended to give you some simple to follow suggestions that will save you many headaches. Unless you have already written industrial strength programs you will find that many of these rules sound overly anal. Trust me, they were born out of many tears and much blood! I don't want to sound like your mother, but much of this is simply a matter of building good habits to avoid a mess later on.
Before going into very specific suggestions we'll start with some general ideas.
void* copy( void* dst, const void* src, const int nbytes ); int* copy( const int* src, int* dst, const int nints );Whichever way you prefer, stick to it! Another example of a useful convention is to spell macros and preprocessor constants in all CAPS.
for(i=obj->begin();i<obj->l()&&obj->M(i)!=-1;i++){ obj->M(i)=-1; }isalothardertoreadthanthefollowingequivalentstatement
for( i = obj->begin(); i < obj->l() && obj->f(i) != -1; i++ ){ obj->f(i) = -1; }Similarly add empty lines between conceptual groups of statements just like you would add paragraphs to a story.
// initialize all faces in obj to empty, starting with the // beginning of the segment up to the maximum length unless // the tail is already initialized to empty for( i = obj->begin(); i < obj->l() && obj->f(i) != -1; i++ ){ obj->f(i) = -1; } // update the begin pointer for the next segment obj->begin() = i;Trust me, it makes large complicated codes much easier to understand to state the ``obvious.''
Obj *obj = new Obj; assert( obj ); void dada( Obj *obj, const int begin, const int end ) { int i = 0; assert( begin >= 0 ); assert( end < obj->length() ); for( i = begin; i < end; i++ ){ obj->f(i) = -1; } } float Angle( const Vec *a, const Vec *b ) { float dot = a[X] * b[X] + a[Y] * b[Y] + a[Z] * b[Z]; float alen = Length( a ), blen = Length( b ); float cosangle = 0; assert( alen > 0 ); assert( blen > 0 ); cosangle = dot / ( alen * blen ); assert( fabsf( cosangle ) <= 1 ); return acosf( cosangle ); }Sometimes it makes sense to use fixed length storage for something that is actually dynamic. For example, if you ``know'' that there'll never be more than 50 vertices that need to change. Be prepared though for the rare input that has more than these:
const int MAXVERT = 50; ... void LoadVertexIndices( int map[MAXVERT], Obj *obj ) { int i = 0; while( obj->NextIndex() != -1 ){ map[i++] = obj->NextIndex(); obj->IncrIndex(); assert( i < MAXVERT ); } }If you don't put an assert() here to alarm you later that you went beyond the limit of something you will trash memory and the symptoms often don't show up until much later in the execution path with extremely hard to pinpoint reasons. These kinds of memory bugs are the nastiest to find!
For other examples see the switch() statement below and the comment on checking return values.
int CallBack( Event *e ) { int result; if( e ){ ... result = 1; ... } return result; }Good luck finding this bug. Only under certain circumstances will this bug be triggered and then only if there happens to be the right kind of garbage on the stack. Those bugs can be reaaaaaal fun.
extern void fish( void );is quite different from
extern void fish();The latter being a non-prototyped function which takes an unspecified set of arguments. Baaaaaad.
switch( i ){ case 'a': ... break; case 'b': ... break; default: assert( 0 ); }
extern int scanf( const char*, ... );clearly states to any user that scanf() will not mess with it's first argument. Nice to know. Similarly the following will be caught:
void dada( const Vec *v ) { ... v->x() = 5; // compiler error }This latter example is trivial, but once your code gets more complicated and you have many levels of indirection you will run into this in places where it is not obvious at all...
i = fscanf( file, "%f %f %f", &x, &y, &z ); assert( i == 3 );
void InitializeFaces( Obj* obj ) { Face *f = obj->faces(); const int nof = obj->length(); const int empty = -1; int i = 0; // set all faces to empty for( i = 0; i < nof; i++ ){ f[i] = empty; } }Note that the initialization of i to zero wasn't necessary here, but it pays to build habits.
Good names for arguments in extern declarations are particularly useful:
extern void VertexMovedCB( Mesh *m, const float newcoord[3], const int meshindex, SoCoordinate3* destcoords );is much easier to understand than
extern void VertexMovedCB( Mesh*, const float*, const int, SoCoordinate3* );
vec3d.h vec3d.cThe header file advertises externally what's user callable in the source file. Having the source file include the header file itself (even though generally this is not necessary) ensures that the source and header never diverge. E.g., you change the type of an argument in the source file, but forget to change it in the header file. This would typically lead to hard to find disasters unless you enlist the compiler's help in pointing out the mismatch.
Once your start programming in OpenInventor you will have to use C++. Aside from the comments on writing style given above there are a few new things to keep in mind.
// forward reference to avoid having to include <iostream.h> here class ostream; class istream; class Color{ public: Color( void ); // default constructor Color( const Color& ); // so-called `X( const X& )' constructor ~Color( void ); // *always* have a destructor Color operator=( const Color& ); friend ostream& operator<<( ostream&, const Color& ); friend istream& operator>>( istream&, Color& ); // section for data accessors (example); float r( void ) const { return p_r; } float& r( void ) { return p_r; } protected: private: // ALL data members should ALWAYS be at least protected if not private float p_r, p_g, p_b; };The default constructor (one that takes no arguments) is needed if you want to be able to declare arrays of a class type. The constructor X( const X& ) is used when calling functions (among other places). Especially for large classes it is profitable to define this constructor explicitly since you can save yourself copy on and off the stack.
Having an explicit destructor for a class, even if it is trivial, really helps building a habit of memory clean programming. Especially in C++ this is really made very simple since all you have to do is make sure that the constructor and destructor do the right thing when it comes to dynamic memory.
The assignment operator is defined by default to do bitwise copy. If you have pointer members of a class this is rarely what you want. Thus it's a good idea to be explicit about it right from the start.
Almost always you'll want a way to print an object to a stream. By defining operator<<(), you can print objects of class type just like any other object with the usual stream semantics. Similarly it is useful to have a read function, operator>>() which should be designed to read whatever the output function generates. For some class types the read function makes little sense, e.g., if pointers are an integral member of a given class.
As a matter of principle all data members should be hidden and any access to them should be mediated through accessors. If the accessors are inline this will not cost you any performance overhead, but if you ever change your mind about the internal implementation of a class, having accessors can often make such a change trivial, rather than having to go through all your code and changing some statements. Note also the use of const in the two accessors above. The first one cannot be used as an Lvalue, while the second one can. You'll typically want both.
class Vector3{ public: Vector3( const float x, const float y, const float z ) : p_x( x ), p_y( y ), p_z( z ) {} Vector3 operator+( const Vector3& v ) { return Vector3( p_x + v.p_x, p_y + v.p_y, p_z + v.p_z; } protected: private: float p_x, p_y, p_z; };Together with a Matrix class your code can be something like:
// create a rotation around the x-axis (with a suitable constructor) Matrix r( 'x', m_pi_f / 6 ); // translation constructor (we assume 4x4 matrices here) Matrix t( Vector3( 1, 1, 1 ) ); // scaling Matrix s( .5f ); // concatenate transforms Matrix m = r * s * t; // transform object for( int i = 0; i < obj->length(); i++ ){ obj->pts( i ) *= m; }Especially when such computations become more involved, i.e., the expressions are more complicated, the C-style of calling a bunch of functions or macros in the right order with manual management of temp variables turns into a mess while the C++ version will (can!) be perfectly readable, and just as efficient!
When first playing with operator overloading many people succumb to overdoing it. Obviously things such as addition and scalar multiplication of vectors make sense and have an obvious operator which should do this. But what do you do about things such as dot products? For example
class Vector3{ public: ... float operator|( const Vector3& v ) { return p_x * v.p_x + p_y * v.p_y + p_z * v.p_z; } protected: private: ... };is very tempting since you can then write angle = a | b. However this is a particularly bad idea. While you can redefine just about every operator in C++ (yes, even the comma operator...), the precedence of operators cannot be changed. Thus your dot product just got the precedence of bitwise OR. This is definitely not what you want (why?).
class Tree{ public: ... protected: private: Tree( const Tree& ) { assert( 0 ); } Tree operator=( const Tree& ) { assert( 0 ); return *this; } ... };this way nobody external to the class can accidentally call these members and if you do it accidentally inside member function code, you'll get an assertion failure. This is a lot better than having the compiler generate a default assignment operator (bitwise copy) and quietly use it...
#define SQR(a) (a)*(a) // versus inline float sqr( const float a ) { return a * a; } // or even better template <class T> inline T sqr( const T& t ) { return t * t; } #define PI 3.14159265358979323846 const double pi = 3.14159265358979323846;The macro SQR simply does text substitution. That has the advantage that it works for any data type which has multiplication defined for it. But, you don't get function call semantics. For example, consider SQR(a*b-c*d). The resulting evaluation will compute the expression a*b-c*d twice, while the function call will first evaluate the expression and assign the result to the actual parameter. Since sqr() is an inline function it will also do the equivalent of text substitution, but with function call semantics, avoiding many of the usual problems with macros. If you want it to work for any suitable data type (like a macro would), you can use the template version of this function.
Similarly, the advantage of replacing preprocessor constants with const imbues the constant with semantics such as scope and type that the preprocessor knows nothing about. For example, you can easily do this:
const double pi = 3.14159265358979323846; ... { const double pi = 3.14; // some computation with the new pi } // original definition back in scope(this example is a bit sick, but you get the idea.)