牵着老婆满街逛

严以律己,宽以待人. 三思而后行.
GMail/GTalk: yanglinbo#google.com;
MSN/Email: tx7do#yahoo.com.cn;
QQ: 3 0 3 3 9 6 9 2 0 .

OGRE Code Review HOWTO

http://temas.obelisk.net/ogre/CR/docs/howto.html

OGRE Code Review HOWTO

Thomas Muldowney


Introduction

The time has come for all good men to come to the aid of their favorite rendering engine. The OGRE Project needs you to help with an ongoing code review. This document will outline how everyone will be able to participate and what guidelines need to be followed. It will also outline what will need to be looked for during the review process.

The Process

The code review will center around this website and use a fairly simple voting process to decide when a file has passed review. Here is a quickstart guide to understanding the process:

  1. Create a normal user account on the website.
  2. Pick a file that needs more reviewers and flag that you are beginning to review it.
  3. With the other reviewers leave comments and/or patches to make the file compliant with the process.
  4. When you are done reviewing a file flag your review complete.
  5. When an administrator has decided you have been helpful in the review process they will promote you to a voter.
  6. Once a voter you can still continue the normal process of commenting and submitting patches, but you can now vote on the completion of a file.
  7. When a file has received a set number of votes it will be flagged for an administrator to finalize.
  8. When an administrator finalizes a file it is considered to be fully satisfying the reveiw rules, and may be reopened when major changes are made.

What to Look For

The following areas are where the focus of the review should be directed. Many will have very specific criteria to be considered complete, but others will be more open to a general concensus of the reviewers. The current list of review points is fairly short and will be updated as the review develops. Many of the review points come from the Effective C++, More Effective C++ and Effective STL by Scotty Meyers. If you don't already have these books, you really want to get them. Other points come from decisions by the core team and practical experience. If you feel there should be additions or changes, please contact .

Style

These rules deal with the general look of the files, including issues such as indenting, bracket placement, variable naming, etc.

  • All indentation is done using 4 spaces, no tabs.

  • Braces are placed under any line that needs a new logical block. This includes if, else, for, while, class defnitions, function definitions, and most others.

    Example 1. Normal Brace Usage

                  while (expr)
                  {
                      // This is the logical block. It spans multiple lines,
                      // and is surrounded by braces.
                  }
                
  • The primary namespace declaration for a file should have the brace placed on the same line as the declaration.

    Example 2. Namespace Brace Usage

                  namespace Ogre {
                
  • Logical blocks that are only one line do not need braces, but should be on a new line and indented. If a previous and connected block statement (else to an if) has braces then the connected block must have braces, even if it is one line long.

    Example 3. One Line Logical Blocks

                  if (expr)
                      // One line block
                  else
                      // else may be handled the same.
                

    Example 4. Connected Blocks

                  if (expr)
                  {
                      // This part is multi line and with braces.
                      // So we must have braces on any connected parts now.
                  }
                  else
                  {
                      // Even though this is on line, it has braces.
                  }
                
  • Control and looping statements that are followed by a "(" should have a space before the open-parenthesis.

    Example 5. Parenthesis Space

                  if (expr)
                      // Space after the if
                  while (expr)
                      // Space after the while
                  for (expr)
                      // Space after the for
                
  • Control and looping expressions inside parentheses should not have a space before or after the expression.

    Example 6. Expression Spaces

                  if (expr) // This acceptable, no space before or after expr
                  if ( expr ) // UNACCEPTABLE, spaces should not be before or after expr
                
  • The "else if" control block should always be on a single line.

    Example 7. Else If Control Block

                  // Acceptable, else if on one line
                  else if
                  // UNACCEPTABLE else if on multiple lines
                  else
                      if
                
  • Switch statements abide by all the previous control block rules, and must have their case statements on the same indentation level as the actual switch statement. Statements inside a case should be indented one level.

    Example 8. Switch Statements

                  // Acceptable
                  switch (expr)
                  {
                  case CASE1:
                      // The case statement is on the same indentation level
                      // with the switch, and it's statements are indented
                      // one level.
                      break;
                  };
    
                  // UNACCEPTABLE
                  switch( expr )
                  {
                      case CASE1:
                          // The case statement should be on the same
                          // indentation level of the switch statement
                          break;
                  };
                
  • All names should be written in English.

  • Method and function names should be verbs that clearly state what they will do. The name should use a mixed case format, starting with a lower case letter. Methods not intended for general API usage, rather for internal engine usage, it should be prefixed with an underscore.

  • Variables should clearly state what information they contain and use a mixed case format, starting with a lower case letter. Private and protected member variables should be prefixed with a lower case "m" and be followed by an upper case letter. The names may use limited abbreviation, but should be quickly understandable. An exception to this is looping variables (such as in afor loop), this may be a simple single letter variable such as "i".

    Example 9. Variable Naming

                    loggingLevel // Most clear
                    logLvl // Acceptable, still quickly clear
                    ll // unacceptable
    
                    for (int i = 0; i < max; ++i) // Acceptable
                  

    Are we sure about the "m" for those member vars?

  • Each class should be contained in its won file, unless the class is directly relevant (supporting) to another and is small.

Design Guidelines

  • Prefer initialization to assignment in constructors.

    This one needs a lot of improvement in our code.

  • Use const when it is possible. This includes variables and method declaration.

  • Use pass-by-reference over pass-by-value when possible.

  • Prefix calls to standard library functions with "::".

  • Strongly prefer C++ style casts to C style casts.

  • For STL containers, use empty() instead of checking size() against zero.

  • When advancing iterators and variables prefer ++var to var++.

  • If a base class has virtual functions, derived classes should have the "virtual" keyword repeated for those functions.

    Note

    There are differing reports on whether this might break some compilations for platforms we still wish to maintain. If that is the case, we need to reevaluate this.

  • Base type names should not use any abbreviations.

    Example 10. Type Names

                  unsigned int // Acceptable
                  uint // UNACCEPTABLE
                
  • C++ style headers should be preferred over C style.

    Example 11. C++ Style Headers

                  #inclue <cstring> // Acceptable
                  #include <string.h> // UNACCEPTABLE
                
  • Prefer standard C++ library and STL functions over C stdlib functions.

    Example 12. Functions

                  // Acceptable
                  ifstream fil;
                  fil.open("filename.txt");
                  fil.close();
    
                  // UNACCEPTABLE
                  FILE* fil = fopen("filename.txt", "w");
                  fclose(fil);
                
  • All class variables should be initialized to a sane value in the constructor.

  • It is not necessary to check that a pointer value is not NULL before deleting it in a destructor, or elsewhere. C++ guarantees that delete on a NULL value is safe.

    Example 13. Deleting Pointers

                    // Acceptable, delete NULL is safe
                    Val* ptrValue = 0;
                    delete ptrValue;
                    // UNACCEPTABLE
                    if (ptrValue != NULL)
                        delete ptrValue;
                
  • 0 should be preferred over NULL.

    Example 14. Prefer 0

                  Val* ptrValue = 0; // Acceptable
                  Val* ptrValue = NULL; // UNACCEPTABLE
                
  • When a member value pointer is deleted, it should be set to 0.

OGRE Specific Guidelines

  • Prefer Ogre::String over std::string or other string types.

posted on 2007-01-29 21:59 杨粼波 阅读(163) 评论(0)  编辑 收藏 引用


只有注册用户登录后才能发表评论。
【推荐】超50万行VC++源码: 大型组态工控、电力仿真CAD与GIS源码库
网站导航: 博客园   IT新闻   BlogJava   知识库   博问   管理