The following coding standards apply to developers working for Fieldpine directly (employees and contractors) on the core Fieldpine products. It does not apply to developers working at customer sites or for external developers coding for customers.
In general, we are not overly precious about the coding standards, however maintaining some common guidelines helps move source code between groups. We do not claim these standards are best practice, but are the way we prefer to see code. If you have a valid reason not to follow these standards, this is typically permitted. (nb, not liking these standards doesn't count as a valid reason)
Cross Language
- Variable names of 1 or 2 characters should be avoided as class members. If a dev sees a variable called "g" or "j2" they will assume it is function local.
- Use sensible variable names. Do not abbreviate just because you can, but be reasonable. MaxCounter is fine, but MaxCnter is not.
- Focus on clarity over conciseness. The developer having to understand your code in several years
is your main priority. For example
if (((x>6)&&(u<67))||(i==2)||((k>98)&&((m==9)||(m>78))) then ...
is much hard to comprehend thanif ( ((x>6)&&(u<67)) || (i==2) || ((k>98) && ((m==9)||(m>78)) ) then ...
but sometimes this is easier stillint bDo = 0; if ((x>6)&&(u<67)) bDo = 1; // comment 1 if (i==2) bDo = 1; // comment 2 if ((k>98) && ((m==9)||(m>78))) bDo = 1; // comment 3 if (bDo) then ...
- Do not overwork optomisation, let the compiler do the work for you. The examples above will all broadly compile to equivalent assembler.
- Comments should be used, but do not put in pointless comments where the code is super obvious. Comments should be more what and why you are doing something rather than how. The code is the how.
- Your main focus is to create code that can be quickly understood by someone in several years time.
- It is acceptable to focus on performance in advance (don't wait for a customer fault call), but concentrate more on the algorithm and structures than attempting to remove 1 assembler instruction.
For C and C++
- Try not to use the comma operator. It is often confusing and easily missed when reading.
- We prefer the following brace style - only because it displays more readable lines on the screen
if (f==6) { f1(); f2(); } else if (f == 9) { f1(); } else { f2(); }
Or,if (f==6) { f1(); f2(); } else if (f == 9) f1(); } else f2();
- Try not too create source lines that are too long, not everyone is using wide screens. But don't wrap a line if it is just a long text string.
- If a class has a static function, try and prefix the function name with "s". For example Sale::sGetSale(...)
- If you use assembler for performance critical areas or other reasons, always provide an equivalent in C as well. Both paths must be tested.
- Use brackets to force precedence and make code clearer to read. For example
if ((h+7)>8) ....
is better thanif (h+7>8) ....
- Macro names should be in uppercase. CHECK rather than Check.
- If writing a loop, try and include a secondary out condition.
Rather thanwhile (p != NULL) { .... p = p->pNext; }
do thisint mx = 4400000; while ((p != nullptr) && (mx-- > 0)) { .... p = p->pNext; } if (p != nullptr) FLOW_ERROR(1234); // NOTE# Loop exited using failsafe rather than expected path
The second form will eventually exit if a loop is ever created by accident (where p->pNext == p). Set the mx variable to value much larger than ever possible, this is not a performance optomisation.
Auto Debugging Support (C & C++)
When creating your code, sprinkle the following macros inside. These aid a variety of automated tools.
Many of these macros expand to nothing in release builds.
The value NNN below is a randomly chosen unique number over the whole program. You might be tempted to use the macro __LINE__ or __COUNTER__ however these are not good choices as the number can change as lines are added or removed. The intent and use of the number is to provide a constant reference point for external tools
Example
if (0==WriteFile(hCom,&wbuf[0],1,&wlen,NULL)) FLOW_ERROR(726122);
Flow commands are not needed around every line, only in places where the flow of execution changes, or where several commands have been executed that could cause an issue. Put another way, if you were given a list of FLOW numbers for a faulty program and nothing else, where would you need FLOW statements. Flow statements would not normally go inside tight loops.
The following is a worked example of a piece of code
COMMTIMEOUTS tout; FLOW(183928); if ( 0==GetCommTimeouts(hCom, &tout)) FLOW_ERROR(81737); tout.ReadIntervalTimeout=200; tout.ReadTotalTimeoutMultiplier=500; tout.ReadTotalTimeoutConstant=500; tout.WriteTotalTimeoutMultiplier=500; tout.WriteTotalTimeoutConstant=500; if (fast) { FLOW(4746); int fasttime = RETAILENV->settings.GetInt("DevScale0FastReadTimer"); if (fasttime<=0) fasttime=200; tout.ReadIntervalTimeout=fasttime; tout.ReadTotalTimeoutMultiplier=0; tout.ReadTotalTimeoutConstant=fasttime; tout.WriteTotalTimeoutMultiplier=fasttime; tout.WriteTotalTimeoutConstant=fasttime; } FLOW(129484); if (0== SetCommTimeouts(hCom, &tout) ) FLOW_ERROR( 636621); CString stype = RETAILENV->settings.GetString("DevScale0Type"); stype.MakeLower(); buffer[0] = 0; TRACEMSG(0, "ScaleType="+stype); if (stype=="ishida vega") { FLOWP(334059); unsigned char wbuf[4]; wbuf[0] = 5; if(0== WriteFile(hCom,&wbuf[0],1,&wlen,NULL) ) FLOW_ERROR( 726112); if(0== ReadFile(hCom,&buffer[0],12,&wlen,NULL), FLOW_ERROR(726113)); ExtractMode=1; } else if (stype=="tec sl47") { /* Bold not used in this block so you can see true appearance */ FLOWP(2123483); unsigned char wbuf[4]; wbuf[0] = 5; if (0== WriteFile(hCom,&wbuf[0],1,&wlen,NULL)) FLOW_ERROR(72997); Sleep(30); // Pause slightly as this scale needs time before receiving next command wbuf[0] = 18; if (0== WriteFile(hCom,&wbuf[0],1,&wlen,NULL)) FLOW_ERROR(726122); if (0== ReadFile(hCom,&buffer[0],12,&wlen,NULL)) FLOW_ERROR(726123)); ExtractMode=1; } else FLOW_ERROR(552525); FLOWP(3747)
If this program is reported as crashing, and the flow history shows ... 183928, 81737, 129484, 636621, 334059. Then it becomes clear that errors had been seen before we finally crashed somewhere in the "ishida vega" block. While we can deduce this from exception handlers and execption the PC, the flow history also allows dynamic profiling and hooks for support programs.