JSSMDX Code Standards
Home Up JSSMDX Products JSSMDX Support Links GotWeek

 

How our standards got started....

    A lot of code today can be generated by wizards. Wizard generated code is never going to be complete, or do exactly what you want it to do all the time - or maybe any of the time. Still, the wizard isn't such a bad way to start writing code. Take what the wizard writes for you, touch it up a bit to suit your own standards' needs and it's going to save you much time over writing everything from scratch. The other good thing about wizards is they are like elephants: they never forget. Can't remember exactly what parameters that message handler needs? The wizard knows, so let it stub out the function for you, and then make it do what you want. Wizard code is for the most part consistent - an important consideration and one of the major reasons for adopting a standard in the first place. So, we'll take a look at what the wizards give us and how and why we change it to end up with readable, maintainable, reliable and solid code.

Header Files

    Use a standard heading at the top of each header file. When you create a new class, the wizard usually generates something that looks like:
    // XParametricCurve2D.h : Declaration of the CXParametricCurve2D class
    //
Sometimes this gets placed at the top of the file, sometimes a couple lines down. Be consistent and move it to the top. This is usually a good start for your heading, especially if you follow good naming conventions for your class objects. The class name should be descriptive enough to tell what the class is for.

    Add your copyright notice. You worked hard writing this stuff, you deserve some credit for it, don't you? OK, so maybe you didn't work so hard and the code is garbage, maybe you don't want your name on it after all, but usually a copyright notice is a good idea.  Use something simple and easy to copy from file to file. 
    // Copyright (C) 2000-Infinity James Shisley Software
    // All Rights Reserved
    //

    Optionally, add some kind of short description about the contents of the file. It doesn't have to be long and windy. Just a few lines about what the functions or classes are for will help anybody else looking at the file.

    If you use source control, it is also not a bad idea to include version comments. These are comments which get updated automatically when you check files in and out of source control. It can include information such as author, current file revision, date last changed, etc. Check your source control system help for details.

Multiple Inclusion

    For header files, always protect against multiple inclusion. This protects against "redefinition" errors if the file is ever included by other header files. The wizard nowadays generates a string based upon the class name and sometimes a guid so it is unique. It is easiest to use what it generates, the long guid string is kind of ugly, but it works. It looks something like this:
    #ifndef __XPARAMETRICCURVE2D_H_
    #define __XPARAMETRICCURVE2D_H_
and down at the end of the file,
    #endif
This #endif is too lonely looking down there at the end of the file all by itself. And what if does it go with? There may be many other #if pragmas in a header file. It is a good idea to always put a comment with the #endif so you can easily tell what if it belongs with. We make it look a bit fancier too, so it is always obvious that this is the end of the header file, so it ends up looking like:
    /////////////////////////////////////////////////////////////////////////////
   
#endif //__XPARAMETRICCURVE2D_H_ -Nothing After Here, Please.
    /////////////////////////////////////////////////////////////////////////////

Included Header Files

   When your header file includes others, add a comment to go with it. It may say something about what is in the other header, or why it is included, or what is being used from the other header. This can help if you ever clean up code and need to know if something is still being used or not.
    #include "resource.h" // main symbols
   
#include "XVectorTypes.h" // STL vector for segment array(s)
It doesn't have to be very complicated, just a short note is usually fine.

Header File Contents

    Group similar functions for helper headers. For class headers, you may want to say only one class per .h/.cpp file, but sometimes this is maybe too restrictive. A class that contains another member class may put them both in the same files.

    Use descriptive names! The worst thing you can do as a programmer is use bad names for functions, variables, or classes. Nothing makes it harder to read code than something named "temp" or "buff", or counters named "i". If you're counting something, like lines, call your counter nLine. It makes it a lot easier to tell what you are counting. Temp is meaningless, I hate Temp. So is Buff. Any local variable is temporary on the stack, and a buffer for some kind of data. Give yourself and your fellow programmers a break and use a name that says what it is really for. Try not to abbreviate too much either. We all hate typing long variable names, but it really makes code a lot more readable to use real complete words rather than abbreviations. How much typing is it going to save you anyway? More about names later.

    Be explicit and be aware of the scope of a function. Helper functions being declared in a header file are being exported .Always declare them as extern. Export only the functions that you need to share with other files. It makes it much easier to change something if you know it is used in other places or just locally. Functions used within a single file should always be declared static, so you can tell right away you don't need to look in other files for it being used.

    Use comments! You can never have too many comments. They may laugh at you, but they can read it! That's more than you can say about their code. Follow every variable declaration with // this is for ...so it is always completely obvious what it is - even if you think it is obvious without the comment. It's going to help somebody who doesn't know what you know, or maybe even help yourself when you have to change it ten years from now. Try to say something about what a function does. This can be very short, probably only a one line description in the header file. Then put more details about the function in the .cpp implementation.

    Typedef is your friend. Make your code easier to read and understand, define types for anything that is going to be the least bit complicated. That includes structures (unions), function pointers, and objects derived from templates. A structure typedef should always define a type for both the structure and a pointer to one. What we prefer is something like:
    typedef struct _SPoint
    {
        double lfX;   
// X-Coordinate Of Point
        double lfY;   
// Y-Coordinate Of Point
    } SPoint, *SPointPtr;
Note, the attention to detail, the {} matching the same level of indentation, the name is a real word "Point", not an abbreviation like Pt, and the comments after each member of the structure to say what it is for. There are other conventions, for example, you may prefer the struct TAG_SPoint or a p for pointer prefix like *pSPoint, these are just as good, as long as whatever you use is consistent. Define function types for callback functions or declaring function pointers:
    typedef int ErrorNotifyCallback(int nErrorCode);    /
/ Callback for error handling
and for templates,
    // Define type for array of curve segments
    typedef vector<CXPCurve2DSegment> CXPCurve2DSegments;
will save time typing the template <> every time and make it easier to read when you declare one of these animals.

Scope

    Member variables should be private - or not ? Some argue that all member variables should be private, and you should use access functions to get or set their values. This is somewhat enforced if you use ATL or COM or Automation and have get/set properties. Sometimes private just gets in the way, and you have to change it later anyway so another class can access the data. Personally, I don't think much protection is gained by hiding variables, especially when get/set functions are going to do simple sets without checking data validity - so I tend to make them mostly public. On the other hand, if you are going to go the extra mile and make sure anything set is valid, and throw some error when it is not, then maybe it is really a good thing. On the other other hand, if you do that, eventually what is "valid" data is going to change, so you have more work to do when it does. You should decide how much time you want to spend to protect your data.

    Member functions should be private when they do not need to be shared with other classes. And don't forget const-ness. If a function is not going to change an argument, make sure it is declared const. Usually this is something we tend to forget.

    Avoid global variables! They are the root of all evil. If you have to have them, and every program has some, wrap them in a global class. The App class in a MFC application is a good place to conceal your global variables, and afford some protection.

Implementation

    Like a header file, a implementation file should also have a standard heading. The wizard gives you something similar to:
    // XParametricCurve2D.cpp : Implementation of CXParametricCurve2D
    //
and add your copyright notice here too. Follow with some general comments about the file contents. It doesn't need a lot of detail at this level, you will do that later for each function.

Order To Chaos

    Decide upon an order for what you put in your source files. Follow it. Separate each section in the file with a standard comment line. It is easiest to copy the line the wizard generates, which is,
    /////////////////////////////////////////////////////////////////////////////
or if you prefer something else, just as long as everybody uses the same separator. It is also a good idea to use this comment line as a guideline for the length other lines of code. Any line of code that extends much longer than the separator line is a candidate for putting it on more than one line. These separators are your friends, they help you page up and down quickly from one section to the next because they are easily recognized as the beginning of a new section. The section order we prefer is:

    Heading (described above)
    Include files
    Local Defines and typedefs
    Local Macros (which might just be included with the define section)
    File Local Global (static) variables (remember Global Global variables are evil)
    Forward Local Functions -exported functions are already declared in the header - and don't declare them all again, there is no need and it just makes the file look cluttered. Only forward what is necessary. Usually it can be arranged so a function appears before it is called, so it needs no forward declaration.
    Implementation Code
    End of file - double separator lines makes it obvious when you're at the end.

Prefer Classes

    Prefer classes over structures and unions. One of the most important benefit is constructors. A good constructor almost always initializes all member variables. This helps prevent un-initialized variable crashes in the release build that take so much time to track down. You can make constructors for structures, but usually no one bothers. It's better to make a class. The other benefit is organization. Classes provide a much better organizational programming model.

    Make a class or classes for local global variables. Grouping them logically according to function is a lot neater than a gazillion variables at the top of every file. It also makes it easier to get them as a group if you need to add an access function for the data later.

Use Descriptive Variable Names

    Nothing is more important. You are not going to bother writing documentation, you are far too lazy. Good variable names can make code nearly self-documenting. The biggest exception may be complicate algorithms that need special mathematical symbols. Then you probably need to write more than just code to explain the algorithm. Usually a good naming convention includes:

bulletPrefix - Hungarian started the whole prefix thing, and it is really helpful to identify variable types. There are variations, decide what you like and use it. We Prefer
    C is for class
    b is for BOOL or Boolean or bool or whatever variant of true and false you care to use
    s is for string (or prefer cs for CString, which is a lot better, especially for UNICODE)
    n is for integers -prefer longs over shorts, shorts invariably overflow
    f is for floats, but prefer doubles
    lf is for doubles
    u is for unsigned
    p is for pointers, pp for pointer to pointer, etc. though going more than pp deep is confusing, it is better to define types for your pointers to make it less confusing
    m_ is for member variables followed by the type then the name
bulletComplete and real words, not abbreviations
bulletCapitalize the first letter of each word in a multi-word variable name
bulletUse all capitals for predefined symbols and/or macros
bulletNEVER EVER call anything TempBuffer (or other variations of temp, tmp, buf,  or buffer), unless it is for a joke

Every Function Must Fail

    There is almost no exception. Plan for the possibility, even if you think something can not fail, eventually it will. I think they call it Murphy's law or something. NEVER discard the value returned from a function. Usually convention dictates that return values indicate some sort of error. Assign the result to a local variable, even if you have no intention at the moment of using the value or passing it up. Someday you will be debugging it when it fails and the result will get lost. You won't have any idea what went wrong without that returned value. Be assured, it will fail if it can. Never assume because a function appears to be working all of the time it won't fail. Just wait until the next version of the OS comes out, you'll see.

    Don't expect memory allocations to succeed. You will run out eventually. Not everybody has the same unlimited resources as your development system.

    Error codes should tell more than I failed. Define meaningful symbols for error conditions, so you can tell what went wrong. Plan on letting the user know what went wrong too. A function that fails and then does nothing is just as annoying and confusing as something that crashes.

Alignment

    Don't be afraid of new lines. They are not going to make your code any slower. When a line gets too long (usually around 80 characters or so) break it into multiple lines. Then you have more room for more comments too - you can remind yourself what the parameters were for, so you don't have to look it up again in the help next time.

    Matching braces should always be aligned at the same level of indentation. All if, for, while, do, and blocks should have braces. The debugger does not always match single line statements well without them, you will get confused. Always use a matching comment at both ends of the brace. It makes it a lot easier to tell what the end brace is for if it has a comment. So it looks stupid on short statement blocks, if you change it later and it gets a lot longer, it will already be there, you won't have to worry about it. If you are making any assumptions about the conditions, put them in the comments so you will remember later what you were thinking, for example,
    if (bBlue == true)
    { // if blue
        ... // do blue stuff
    } // if blue
    else
    { // not blue - well then it must be black
        ...
    } // not blue

    Use spaces and don't crowd everything together. It is a lot easier to read. Spaces don't make your code run any slower either.

    There are two different types of switch statement alignment we prefer to use. The first is for a simple mapping switch statement that maps one variable to another, for example, to map a error code to the appropriate error message string ID. For the mapping switch, one line per item:
    switch (nError)
    { // switch on error code
        case Err1:        nErrMsg =IDS_ERROR1;    break;
        case Err2:        nErrMsg =IDS_ERROT2;    break;
        default:            nErrMsg =IDS_ERROR_UNKNOWN;    break;
    } // switch on error code

    Note for switch statements, the switch braces always have a comment that indicates what the switch is about, and shows where the end of the statement lies. Switch statements always should have a default - even if it  just contains a TRACE("Unknown case in function XYZ")". This is extremely useful for tracking down those places where somebody forgot to add the new value to the switch.

    The other type of switch statement we recognize does more work for each case. Each case should have braces and matching comments. A line between each case makes it easier to read.  Rewriting the example above to use this format looks like this:
    switch (nError)
    { // switch on error code
        case Err1:
        {
            nErrMsg =IDS_ERROR1;
            break;
        } // Err1

        case Err2:
        {
            nErrMsg =IDS_ERROT2;
            break;
        } // Err2

        default:
        {
            nErrMsg =IDS_ERROR_UNKNOWN;
            break;
        } // default
    } // switch on error code

Function Headings And Footer

    Each function has a heading. It starts with the separator line. It may and probably should include any and/or all  of the following:
    Name and short description of the function
    Description of the arguments to the function, whether they are input parameters or output results
    What values can be returned from the function and the conditions that cause failures

    The footer is the ending brace, followed by a comment, of course, that is simply the name of the function. This helps you when you are scrolling up between functions - when you can see a separator, you can always see the name of the function above and below your current position. 

Comments

    You can never have too many comments. Prefer // to the end of line comments  over /* */ even if a comment is longer than a single line. The reason is, it makes it easier to then temporarily comment out blocks with /* */ that include other comments.

Rules For Declaring Local Variables

    Use one line for each variable declaration.

     If it is not a class variable, i.e. it has no constructor, always assign initial values when it is declared.

    Put a comment that says what the variable is used for.

    Example:
    long nItem =0;    // Counter for the item indexed in the snerf list

See Ya

    OK, so now you write perfect code. Your program is never going to crash ever again. Now you can spend your time writing new things instead of listening to everybody complain. Or now, instead of fixing bugs, you have time to take that vacation to Tahiti! Have fun, see you there....