Beijer Electronics (formerly QSI Corporation)

Manufacturer of Mobile Data and Human Machine Interface Terminals.
It is currently Thu Nov 23, 2017 11:24 am

All times are UTC - 7 hours




Post new topic Reply to topic  [ 18 posts ]  Go to page 1, 2  Next
Author Message
 Post subject: Coding Standard
PostPosted: Thu Jan 03, 2008 10:22 am 
Offline
QSI Support
QSI Support
User avatar

Joined: Wed Mar 08, 2006 12:25 pm
Posts: 881
Location: Salt Lake City, Utah
I thought a discussion on the Coding standard might be in order as it has been a number of years since we have touched it.



I would like to lead out with the following thoughts:



Secion 10: Limit lines to 80 characters. This made since back in 99, when we had people who were still running 800x600 resolution. However, this is 2008 and I think we all have 1280x1024 by now and have dual monitors on top of that. I know that I can easily get 150+ characters without scrolling.



I would suggest we bump that to 120 or even 140.







Section 5:
Quote:
In general, all variables should be declared at the beginning of a function implementation or an

individual block




Suggest that we remove this restriction. It is needless in C++ and runs counter to the current feeling by most experts who recommend instantiating variables near their usage rather than at the top of blocks.



This can also be overly restrictive in C++ where you may not know the arguments to pass to a constructor at the top of a block, as the construction arguments may be calculated in the code of that block.





Suggest we allow the use of #pragma once in code that is strictly targeted for Microsoft compilers, instead of #ifndef HEADER_H / #define HEADER_H.

_________________
Jeremy
http://www.beijerinc.com


Top
 Profile  
 
 Post subject:
PostPosted: Thu Jan 03, 2008 11:39 am 
As a comment to your first suggestion, there are still people who use editors that show text in two windows side by side (I don't often use the side by side view in emacs, but occasionally I do). Therefore, even the 80 character restriction is a bit high. However, occasionally I do run into lines that just won't break up reasonably into anything less than about 100 characters. I've always been a proponent of "Use Your Best Judgement" decision making, so perhaps we could soften the tone of this one to say something like "Try to limit lines to 80 characters or less". Most lines should not extend past this range, but some will. Another reason cited by Jean Labrosse (uC/OS book) is that more than about 80 characters won't print on an 8.5" by 11" page (pg 191 - that is also why he dislikes using tabs (pg 201)).



I actually disagree with the second comment that we should allow variable declaration everywhere instead of in the block that they are used. The basis of my position rests on the fact that once the variable is declared it does not go away unless the block is terminated. If you want to add variables in the middle of a function (or in an "if" statement like I commonly do), create a new block (the "if" statement creates one by default) and declare the variables there. That way the scope of the variable is easily identifiable and we don't have to try to dig through the code to figure out where the variable came from (parameter, global, or local).



I've always kind of liked the way Labrosse put it (pg 190), "There are many ways to code a program in C (or any other language). The style you use is just as good as any other as long as you strive to attain the following goals: Portability, Consistency, Neatness, Easy maintenance, Easy understanding, Simplicity." Rules are only useful to the extent that they are followed. Sometimes suggestions work better. And of course, psychologically it always helps to have a rational explanation of why your doing it that way.


Last edited by ryan on Thu Jan 03, 2008 5:47 pm, edited 1 time in total.

Top
  
 
 Post subject: line character limit
PostPosted: Thu Jan 03, 2008 1:37 pm 
Offline

Joined: Thu Jan 03, 2008 1:17 pm
Posts: 3
As Ryan mentioned about the number of charcters that can fit on a 8.5 x 11 paper, I know that in Studio it can hold 94 characters with the standard printing font and size.



As for tabs, there was no mention of a standard if we should insert spaces or preserve tabs. Should we specify a default tab size? I change studios default from 4 to 2 characters.


Top
 Profile  
 
 Post subject:
PostPosted: Thu Jan 03, 2008 1:43 pm 
Offline
User avatar

Joined: Thu Mar 02, 2006 2:12 pm
Posts: 487
Location: Salt Lake City, Utah
The current C/C++ coding standard specifies four spaces for tabs (page 1, Indentation).

_________________
Ron L.

http://www.beijerelectronicsinc.com/


Top
 Profile  
 
 Post subject:
PostPosted: Thu Jan 03, 2008 1:50 pm 
Offline
QSI Support
QSI Support
User avatar

Joined: Wed Mar 08, 2006 12:25 pm
Posts: 881
Location: Salt Lake City, Utah
The standard said preserve tabs and set them at 4 characters.



Does anyone here actually print out source code? The side by side argument has some legitimacy, however with dual monitors the standard here and the limited amount of time most of us spend looking at side-by-side code, I would say the convenience of longer lines should win out (just my opinion, feel free to disagree)



On variable declaration in C++, keep in mind that in many (most?) cases, declaring an item involves additional code of calling constructors, allocating memory and such. If the function could return before the variable is used that is wasted time/space. Largely a non-issue on PCs, but could be important for the embedded programmers here.



Usually, when I see a code section offset in braces, I tend to think that it has some special reason to be that way, not simply to keep variable declarations near the code usage site.



Code:

string GetNakedFilename (const string &fullPathName)



    if (!IsValidPath(fullPathName)) {

        return "";

    }





    {

         CHAR drv[MAX_DRIVE];

         CHAR path[MAX_PATH];

         CHAR name[MAX_NAME];

         CHAR ext[MAX_EXT];

 

         if (!_splitpath(drv, path, name, ext, fullPathName)) {

            return "";

         }



         {

             string result(name + string(ext));

             ModifyCaseToMatchActualCaseOnDisk(result);

             return result;

         }

    }

}





This example is an abomination to me. All that extra scoping is a waste. All variables could be declared at the top of the function, but again seems like a waste. Especially because result's constructor is needlessly executed.



Also, you can really get indented pretty far if your function gets much more complex than this. (which makes the 80 characters really begin to seem miserly).



Code:

string GetNakedFilename (const string &fullPathName)



    if (!IsValidPath(fullPathName)) {

        return "";

    }





     CHAR drv[MAX_DRIVE];

     CHAR path[MAX_PATH];

     CHAR name[MAX_NAME];

     CHAR ext[MAX_EXT];

 

     if (!_splitpath(drv, path, name, ext, fullPathName)) {

        return "";

     }



      string result(name + string(ext));

      ModifyCaseToMatchActualCaseOnDisk(result);

      return result;

    }

}





is much nicer in my mind.

_________________
Jeremy
http://www.beijerinc.com


Top
 Profile  
 
PostPosted: Thu Jan 03, 2008 2:21 pm 
Offline

Joined: Thu Jan 03, 2008 1:17 pm
Posts: 3
What are the reasons why braces are:



Code:

if () {

} else if () {

} else {

}

while () {

}





I avoid this for the following reasons:



Code:

if (the_open_brace == (somewhere_on & this_line)) {

    your_eyes_are_busy = looking_for_the_open_brace;

}



if (the_open_brace == (one_line & below))

{

    your_eyes_easily_find = the_open_brace;

}



if (brace_at_end == true) {

    is_easy_to_forget = true;

is_easy_to_forget = false;

your_are_still_in_the_if = true;

oops.wasted_time();

}



Top
 Profile  
 
 Post subject:
PostPosted: Thu Jan 03, 2008 2:27 pm 
Offline
User avatar

Joined: Thu Mar 02, 2006 2:12 pm
Posts: 487
Location: Salt Lake City, Utah
I noticed that the .cpp files in the firmware code have a similar style as the .h file in Appendix B. Does this need to be added to the style guide? Should we do .cpp files this style?



Code:
/*******************************************************************/

/*                                                                 */

/* File:  substatements.cpp                                        */

/*                                                                 */

/* Description: This file contains the implementation of the       */

/*              PCObj structures declared in substatements.h       */

/*                                                                 */

/*                                                                 */

/*******************************************************************/

/*      This program, in any form or on any media, may not be      */

/*        used for any purpose without the express, written        */

/*                      consent of QSI Corp.                       */

/*                                                                 */

/*             (c) Copyright 2000 QSI Corporation                  */

/*******************************************************************/

/* Release History                                                 */

/*                                                                 */

/*  Date        By           Rev     Description                   */

/*-----------------------------------------------------------------*/

/* 28-Apr-00  RNM/JBR        1.0     1st Release                   */

/*******************************************************************/



/*****************************/

/* Standard Library Includes */

/*****************************/



/*******************************/

/* Programmer Library Includes */

/*******************************/

#include "basictypes.h"

#include "errors.h"

#include "token.h"

#include "parsecode.h"

#include "programelts.h"

#include "ir.h"

#include "substatements.h"



/***********/

/* Pragmas */

/***********/



/***********/

/* Defines */

/***********/



/************/

/* TypeDefs */

/************/



/**********/

/* Macros */

/**********/



/********************/

/* Global Variables */

/********************/



/************/

/* Routines */

/************/





//-------------------------------------------------------------------------

/*******************************/

/* PCExpression::~PCExpression */

/*******************************/

PCExpression::~PCExpression() {

   switch (valid) {

   case VvalVal:

       delete value.valVal;

       break;

_________________
Ron L.

http://www.beijerelectronicsinc.com/


Top
 Profile  
 
 Post subject:
PostPosted: Thu Jan 03, 2008 2:36 pm 
Offline
QSI Support
QSI Support
User avatar

Joined: Wed Mar 08, 2006 12:25 pm
Posts: 881
Location: Salt Lake City, Utah
The logic behind the braces is as follows:



The decision was made long ago that for block statements (like if) we will ALWAYS surround the code in braces.



Once you accept that, you no longer bother looking at the end of the line -- you know that the brace is there.



You also no longer forget that you are in a code block -- the closing brace must exist.



IIRC, the decision to put the brace at the end of the line was a bit arbitrary, but was intended to keep small 1 line if statements from occupying 4 lines.

_________________
Jeremy
http://www.beijerinc.com


Top
 Profile  
 
 Post subject:
PostPosted: Thu Jan 03, 2008 5:44 pm 
I think this reads much better (although I would use direct comparisons in the if statement for my personal style):



Code:
string GetNakedFilename (const string &fullPathName) {

   static CHAR drv[MAX_DRIVE];

   static CHAR path[MAX_PATH];

   static CHAR name[MAX_NAME];

   static CHAR ext[MAX_EXT];

     

   if (!IsValidPath(fullPathName)) {

      return "";

   }



   if (!_splitpath(drv, path, name, ext, fullPathName)) {

      return "";

   } else {

      string result(name + string(ext));



      ModifyCaseToMatchActualCaseOnDisk(result);

      return result;

   }

}




I do dislike the idea that result is passed in as a string and yet is apparently modified by a function. At least the name of the function indicates it is going to modify something, but I would think a return value would be nicer than an input parameter modification (in case you wanted to keep the actually passed in value around to display to the user as what he actually typed in). Alright - flame on mes amis :D!


Top
  
 
 Post subject:
PostPosted: Fri Jan 04, 2008 9:55 am 
Offline

Joined: Fri Mar 10, 2006 9:53 am
Posts: 6
Location: QSI Corporation
Regarding the 80 character recommendation:



Personally I use the side-by-side view all the time (with different files or multiple views of the same file in different locations). Emacs and Brief make this very easy and convenient. Exploiting multiple monitors for this requires either multiple instances of the editor or an awkward split across the screens (nevertheless, I always find something useful to display on the second monitor).



Regarding the brace conventions, IMO this was probably the most significant convention that we put into the standard. Naked blocks (one-liners with no braces) are very bug-prone, and the braces emphasis the block structure of the code. This has become so ingrained for me that naked blocks now stand out and just don't look right to me.



The opening brace was placed on the line with the "if" (or "else" or "while" or whatever) to conserve vertical space.



Tom, I'm unconvinced by your argument. The situation you describe is not a problem if your editor supports auto-indentation, which (with syntax highlighting) I would consider essential for any program editor.

_________________
Michael K. Elwood

Senior Design Engineer

QSI Corporation

http://www.qsicorp.com


Top
 Profile  
 
 Post subject: Function comment headers
PostPosted: Fri Jan 04, 2008 1:23 pm 
Offline
Site Admin
Site Admin
User avatar

Joined: Wed Mar 08, 2006 12:35 pm
Posts: 31
Location: Salt Lake City, Utah
Section 3 of the coding standard specifies that a "short header containing the function name" should precede the function declaration or definition be included, e.g.:



Code:
/****************/

/* functionName */

/****************/






1. This header, especially included without any other comments, is redundant and useless. The name of the function is already provided in the declaration or definition just two lines down. Should the function name need to change, both copies must be updated, providing the potential for the comment and declaration/definition to fall out of sync.



2. Comment #1 still applies when additional comments in the function header are provided, such as parameter descriptions and a functional description. The function name is redundant.



3. If the requirement is a visual separation of one function from another, then I would suggest:

Code:
/*********************************************/


or even better:

Code:
//*********************************************




4. A comment containing the function name can be useful when placed at the end of a function that is longer than a screen length. E.g:



Code:
void myFunc()

{

    ...a bunch of code longer than a screen length...

}  // myFunc




This comment helps when browsing through a file and only the lower portion of the function is visible. It prevents having to scroll up (or match braces or whatever) to find the name of the function.



I would suggest the use of this type of comment as a optional entry in the coding standard. It has the same maintenance issues (could fall out of sync), but at least it is useful.

_________________
Brian Crofts

QSI Corporation

801-466-8770 x431

http://www.qsicorp.com


Top
 Profile  
 
 Post subject: Indentation
PostPosted: Fri Jan 04, 2008 2:09 pm 
Offline
Site Admin
Site Admin
User avatar

Joined: Wed Mar 08, 2006 12:35 pm
Posts: 31
Location: Salt Lake City, Utah
Section 2 of the coding standard specifies that indentations should be created with tab characters and not spaces.



What is the justification for this statement?



Using tab characters has the disadvantage that the indent spacing will look different on different editors, depending on the particular editor's tab spacing. Using spaces for indentation avoids this issue and provides a consistent look and feel, no matter what editor you happen to be using.



The only counterargument that I can think of is that spaces consume more hard disk space. This issue is mitigated with today's hard disk capacities and the ability for cvs to compress on commit.



I would suggest that indentations should always be made with spaces and not tab characters.

_________________
Brian Crofts

QSI Corporation

801-466-8770 x431

http://www.qsicorp.com


Top
 Profile  
 
 Post subject: Re: Indentation
PostPosted: Fri Jan 04, 2008 2:12 pm 
brianc wrote:

I would suggest that indentations should always be made with spaces and not tab characters.




I'm with you - never have liked that particular one myself.


Top
  
 
PostPosted: Fri Jan 04, 2008 4:08 pm 
Offline
Site Admin
Site Admin
User avatar

Joined: Wed Mar 08, 2006 12:35 pm
Posts: 31
Location: Salt Lake City, Utah
I suggest that we add a section to the coding standard that provides guidelines for public and private interface specifications.



These would be helpful mostly for C, since C++ has internal mechanisms for handling private vs. public.



1. Definitions:

    A module is a collection of source and header files that comprise a logical group or function, e.g. the tcpip library, or the set of source and header files used for feedback.

    A public function or variable may be used outside of the module.

    A private function or variable is used only within the module.

    An interface is a a set of public functions and variables that are used to interact with a module.


2. Private functions and variables must be declared static if they are used within only one source file. Declarations for these functions and variables may be located only inside the source file. In general, declare stuff only where it is used -- don't put it in a header file if it is not needed by other source files. Try to limit scope where possible.

3. Public functions and variables must be declared as extern in a public header file.

4. No public header can include a private function or variable declaration. They may only include public function and variable declarations.

5. Public functions and variables should follow a consistent naming convention, e.g. all have the same name prefix. This helps with identifying usage of the public interface.

6. Function comment headers for public functions and variables should be placed next to the declarations in the public header file, NOT next to the definitions in the source file. This is akin to documenting a class declaration in a C++ header file.

7. Public header files should be named with a "Public" suffix, e.g. "etherPublic.h".

8. Optional: add a "Private" suffix to header files that are not public header files.

9. Optional: add a "Public" suffix to source files that contain public function and variable definitions; same with "Private" for those that don't.

_________________
Brian Crofts

QSI Corporation

801-466-8770 x431

http://www.qsicorp.com


Top
 Profile  
 
 Post subject:
PostPosted: Fri Jan 04, 2008 4:19 pm 
Offline

Joined: Fri Mar 10, 2006 9:53 am
Posts: 6
Location: QSI Corporation
Brian, regarding the "function name header", I completely disagree with you. It is neither redundant nor useless, because it clearly delimits the functions in a source file. Contrary to your rather arbitrary assertion in your comment 2, tt is especially helpful when you are prepending comments to a function (to describe parameters, return value, whatever) because it binds those comments to the function, instead of just floating them above a declaration.



I would contend that the function name is not strictly redundant because it provides a quick visual reference to the function name; your eyes don't have to parse the name out of a potentially complex declaration statement (syntax highlighting notwithstanding).



The maintenance burden is not egregious, since (a) most of us don't rename functions very often, and (b) the header is in close proximity to the function name, should you feel the need to change it. On the other hand, your proposed "function name comment after the closing brace" has much greater separation, and thus a much higher probability of creating a maintenance issue.



I've never been a big fan of appending comments to close-braces; they signify the end of a block - no more or less. Indentation tells you which block is closing. Unindented braces following code are very clearly end-of-function braces. If you are unsure what function is ending there, most editors provide a means to navigate quickly between matched braces. IMHO that's no big deal. Perhaps others will disagree.



The proposed delimiters in your comment #3 are IMO just too ugly to consider, and lose the utility of the function header comment.



Thoughts, anyone? Please speak up.

_________________
Michael K. Elwood

Senior Design Engineer

QSI Corporation

http://www.qsicorp.com


Top
 Profile  
 
Display posts from previous:  Sort by  
Post new topic Reply to topic  [ 18 posts ]  Go to page 1, 2  Next

All times are UTC - 7 hours


You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot post attachments in this forum

Search for:
Jump to:  
cron
Powered by phpBB® Forum Software © phpBB Group