Art & Logic Programming Style Guide: Formatting

Indentation

[fmt01] All indentation is done using space characters (0x20) not literal tabs (0x09), using three spaces per logical tab stop. Set your editor to always save files in this format.

[fmt02] Preprocessor directives and labels used as the target of a goto statement are always placed in the first column, regardless of the current ambient indentation level:

   bool ok = true;
   for (SInt32 i = 0; i < kSomeConstant; ++i)
   {
      for (SInt32 j = 0; j < kSomeOtherConstant; ++j)
      {
#ifdef qVerbose
         std::clog << "testing: " << i << ", " << j << std::endl;
#endif
         ok = this->Test(i, j);
         if (!ok)
         {
            std::cerr << "Fatal error testing: " << i << ", " << j << std::endl;
            goto CleanUp;
         }
      }
   }
   return true;

CleanUp:
   // cleanup code here...
   return false;

Other constructs are indented as outlined below.

Line Length

[fmt03] Avoid using source lines longer than 79 characters. Occasional violations of this item are acceptable when the resulting code avoids an awkward line break or is otherwise more readable as a single long line. Very long lines due to deep indentation levels may be a sign that deeply nested code should be refactored out into a separate function.

Line Wrapping

[fmt04] Logical lines that are longer than 81 characters should be broken at the most logical point and continued on the next line, indented an additional single space past the current indent level, like:

            1         2         3         4         5         6         7
   1234567890123456789012345678901234567890123456789012345678901234567890123456789
   retval = FunctionWithManyArguments(longArgumentName1, longArgumentName2,
    longArgumentName3, longArgumentName4);

An alternative layout that's taller rather than wider is:

            1         2         3         4         5         6         7
   1234567890123456789012345678901234567890123456789012345678901234567890123456789
         retval = FunctionWithManyArguments(longArgumentName1, 
          longArgumentName2,
          longArgumentName3, 
          longArgumentName4);

or:

   retval = FunctionWithManyArguments(
    longArgumentName1, 
    longArgumentName2,
    longArgumentName3, 
    longArgumentName4
    );

The nice thing about using this tall layout is that it makes it easy to add comments that identify each of the arguments made to a function with a large number of parameters:

   retval = FunctionWithManyArguments(
    longArgumentName1,     // filename
    longArgumentName2,     // access mode
    longArgumentName3,     // share mode
    longArgumentName4      // security
    );

[fmt05] Lines containing expressions should be broken as naturally as possible. Where possible, break after a comma:

   retval = anObject.SomeUsefulFunction(x.LeftMargin() + x.RightMargin(),
    x.TopMargin() + x.BottomMargin() - 1);

If that's not appropriate or feasible, break after an operator:

   retval = anObject.SomeUsefulFunction(x.LeftMargin() + x.RightMargin() +
    anObject.HorizontalPadding(), x.TopMargin() + x.BottomMargin() - 1);

[fmt06] Only use the \ backslash line-continuation mechanism when it's not possible to find a cleaner place to break. As McConnell puts it in Code Complete:

Large-scale Statements:

if/else

[fmt07]

if (someBoolean)
{
   // code to execute when true
}
else if (someOtherBoolean)
{
   // some other code to execute
}
else
{
   // code to execute in other cases...
}

for()

[fmt08]

   for (int i = 0; i < kSomeConstant; ++i)
   {
      // loop code here...
   }

Note that we prefer to use the pre-increment ++i as a general habit. It is guaranteed in all languages that use this construct to work identically to the more commonly seen post-increment i++ that dates back to the earliest days of the C language. In C++, however, maintaining this habit prevents possible inefficiencies when looping over an object that exposes an iteratable interface. Consider the likely cost difference between:

typedef std::vector MyVector;
MyVector vec;
// assume the vector gets filled here....
for (MyVector::const_iterator i = vec.begin(); i != vec.end(); i++)
{
   // clever and important code here...
}

and:

for (MyVector::const_iterator i = vec.begin(); i != vec.end(); ++i)
{
   // even more clever and important code here...
}

Yes, as Knuth said:

but one of the benefits of building up habits is that they let us forget things, but still obtain their advantages.

while()

[fmt09]

while (someBoolean)
{
   // useful code...
}

If the body of the while() is empty, make that explicit, as:

while (someFuncReturningBool())
{
   // intentionally empty
}

not:

while (someFuncReturningBool());

do-while()

[fmt10]

do
{
   // loop code here...
} while (someBoolean);

switch

[fmt11] Case statements inside a switch statement are indented one tab stop, and the code for each case is at the same indentation level:

switch (midiEventType)
{
   // comments precede the case
   case kNoteOn:
   this->NoteOn(now);
   if (duration > 0)
   {
      this->NoteOff(now + duration);
   }
   break;
   
   case kNoteOff:
   this->NoteOff(now);
   break;
   
   case  kPolyAfter:
   after = channelValue;
   // break;            /// include, but comment out the break when falling through intentionally.
   
   case kChannelAfter:
   this->AfterTouch(after);
   break;
   
   case kPitchbend:  // no need to include a commented out break when falling through an empty case
                     // blank line...
   default:
   ASSERT(false);
   this->LogError("Unsupported event type!");
   break;
}

An alternative layout to the above follows, which places the body of each case handler in its own scope. Not only is this more attractive visually, it provides an easy way to declare local variables that are restricted to the scope of an individual case:

switch (midiEventType)
{
   // comments precede the case
   case kNoteOn:
   {
      this->NoteOn(now);
      if (duration > 0)
      {
         this->NoteOff(now + duration);
      }
   }
   break;
   
   case kNoteOff:
   {
      this->NoteOff(now);
   }
   break;
   
   case  kPolyAfter:
   {
      after = channelValue;
   }
   // break;            // include, but comment out the break when falling through intentionally.
   
   case kChannelAfter:
   {
      this->AfterTouch(after);
   }
   break;
   
   case kPitchbend:  // no need to include a commented out break when falling through an empty case
                     // blank line...
   default:
   {
      ASSERT(false);
      this->LogError("Unsupported event type!");
   }
   break;

[fmt12] Switch statements should always have a default case. If the default case should never be executed, place an ASSERT(false); to make sure that this error is caught during debugging and testing.

[fmt35] When intentionally 'falling through' the bottom of a switch case, include the break; inside of comments, along with a comment indicating that this in done intentionally to avoid maintenance programmers from 'correcting' this 'mistake'.

[fmt36] The exception to [fmt35] is when you're falling through an empty case; there's no need to include a commented-out break. Separate the two case labels with a blank line in the source.

Small-scale

Declaring and Initializing Variables

[fmt13] Declare variables as close to the point of first use as possible, rather than in a single big block at the top of the current scope (unless you're programming in C).

[fmt14] Only declare one variable per line of source code -- don't do this:

   // NOTE that the following line of code is probably not what the programmer
   // intended to write, as 'xPos' is a pointer-to-UInt8, but 'yPos' is a
   // UInt8.
   UInt8* xPos, yPos;
   double yaw, pitch, roll;

instead, do:

   UInt8* xPos = NULL;
   UInt8* yPos = NULL;
   
   double yaw = 0.;
   double pitch = 0.;
   double roll = 0.;

[fmt15] Avoid leaving variables uninitialized to avoid the possibility of errors resulting from those variables being accessed while filled with garbage.

Braces

[fmt16] Matching curly braces should always be aligned in the same column. In general, this means that curly braces should always be on a line by themselves:

   if (someBoolean)
   {
      someVariable += 1;
   }

not:

   if (someBoolean) {
      someVariable += 1;
   }

[fmt17] All code inside the braces is indented one tab stop (which is effected by using three space characters).

[fmt18] The only exception to the 'braces alone on a line' rule is found when using a do/while loop:

   do
   {
      //  some intelligent and useful code here...
   } while (someCondition);

In this construct, placing the closing brace and the while() on the same line make it clear that the while expression is tied to the preceding scope, which would not be otherwise visually obvious.

[fmt19] In general, prefer to use curly braces even in situations where the language does not require their use, such as very simple if/else statements or for loops:

   if (someBoolean)
   {
      functionCall();
   }

   for (int i = 0; i < kLimit; ++i)
   {
      fBuffer[i] = 0;
   }

not:

   if (someBoolean)
      functionCall();

   for (int i = 0; i < kLimit; ++i)
      fBuffer[i] = 0;

to avoid maintenance problems in the future.

[fmt20] Another case where "unnecessary" braces should be used is when writing an empty while loop:

   while (*p++ = *q++)
   {
      // this loop intentionally left empty...  
   }

instead of the form that is more commonly found in the wild:

   while (*p++ = *q++);

By prohibiting this common loop format, we can easily check for cases where legal (but wrong) code like:

   int i = 0;
   while (i++ < kLoopLimit);
   {
      myBuffer[i] = 0;
   }

performs a loop with no body, then executes the intended body of the loop exactly once. Python programmers can stop chuckling now.

Whitespace

Bitwise Operators

[fmt31] Always use hexadecimal constants or literal values when working with bitwise operators:

   // see if this is a MIDI status or data byte...
   if (dataByte & 0x80)
   {
      // handle status byte
   }

never:

   // see if this is a MIDI status or data byte...
   if (dataByte & 128)
   {
      // handle status byte
   }
   

Explicit is Better Than Implicit

[fmt32] Don't rely on the fact that many operations may silently be coerced by the compiler into boolean values. Always make the comparison explicit:

   if (pointer != NULL)
   {
      // code...
   }

not:

   if (pointer)
   {
      // code
   }

[fmt33] However, if you are in fact working with real boolean values, there is no need to labor the point -- code like:

   if (truthVal)

or:

   if (!truthval)

is fine -- you don't need to write code like::

   if (true == truthVal)

or:

   if (false == truthval)

Avoiding Unintended Assignments

[fmt34] "Yoda Conditions" -- To avoid a common bug, always place a constant before a variable when performing equivalence testing:

   if (kSentinelValue == val)

instead of:

   if (val == kSentinelValue)

Note that this 'constants come first' rule only applies to equivalence testing, so code like

   if (val < kSentinelValue)
   {
      // ...whatever...
   }

is fine.

Previous: Commenting

Up: StyleGuide

Next: Rosettas