-
Notifications
You must be signed in to change notification settings - Fork 378
Coding Standards Part 1 Style
Formatting guidelines are not overly strict. The important thing is that code is clear and readable with an appropriate amount of whitespace and reasonable length lines. A few best practices are also mentioned.
Tabs are not allowed, and a mixture of tabs and spaces is strictly forbidden. Modern autoindenting IDEs and editors require a consistent standard to be set. In EnergyPlus, we are using 4 spaces per indentation level. Set your IDE appropriately.
int myFunction(bool b)
{
if (b)
{
// do something
}
}[SM] There are good arguments for both tab and space indenting. Tabs have some benefits: 1) User controls indenting they want to see in their IDE, 2) With visual indent set to 3 or above commenting out lines doesn't break indentation alignment (this is very nice!), 3) Files are slightly smaller and thus should be very slightly faster to compile.
[JT] Consistency is the most important thing here. Tabs seem to have stuck at this point, so that is where we are
Code blocks must be enclosed by braces, i.e., {}. C/C++ allow one-line code blocks to be used without braces, but this can sometimes lead to hard-to-find bugs. So braces it is.
// Bad Idea
// this compiles and does what you want, but can lead to confusing errors if close attention is not paid.
for (int i = 0; i < 15; ++i)
std::cout << i << std::endl;
// Much better and readability is not impacted
// It's clear which statements are part of the loop (or if block, or whatever)
for (int i = 0; i < 15; ++i) {
std::cout << i << std::endl;
}Comment blocks should use //, not /* */. Using // makes it much easier to comment out a block of code while debugging.
// this function does something
int myFunc()
{
}To comment out this function block during debugging we might do:
/*
// this function does something
int myFunc()
{
}
*/which would be impossible if the function comment header used /* */.
For large code blocks, it's good to add a comment to the closing brace to identify the code block that it is closing. Here are some examples.
int myFunction()
{
for (int i = 0; i < max_i; ++i) {
for (int j = 0; j < max_j; ++j) {
for (int k = 0; k < max_k; ++k) {
if (array[i*max_k*max_j + j*max_k + k] > 0) {
// do a bunch of stuff
} // if (array[] > 0)
} // for (k)
} // for (j)
} // for (i)
} // myFunctionIsn't that better?
Keep lines to a reasonable length to avoid wrapping and improve readability.
[SM] Converted code is currently unwrapped pending decision on line wrapping. Since IDEs can soft-wrap for you one option is to leave lines unwrapped and let the user control visual wrapping to make best use of their display and preferences. Wrapping lines is hard to do nicely and some editors (looking at you, emacs!) do silly things with aligning wrapped lines on function parentheses that creates a maintenance burden.
There are several "rules of thumb" for helping with this.
If a function has more than one or two parameters, it is a good idea to put each parameter on a separate line. For example, this is not very readable:
void CalcSecondaryAirOutletCondition(EnergyPlusData &state, int EvapCoolNum, OperatingMode OperatingMode, Real64 AirMassFlowSec, Real64 EDBTSec, Real64 EWBTSec, Real64 EHumRatSec, Real64 QHXTotal, Real64 &QHXLatent);This is much better:
void CalcSecondaryAirOutletCondition(EnergyPlusData &state, // this format also lets you use end-of-line comments to document the parameters
int EvapCoolNum,
OperatingMode OperatingMode, // using a variable name that is the same name as a type is not great
Real64 AirMassFlowSec,
Real64 EDBTSec,
Real64 EWBTSec,
Real64 EHumRatSec,
Real64 QHXTotal,
Real64 &QHXLatent); // This should probably be a return value rather than a parameterYou can also do this with function arguments when calling a function, although this should be necessary in fewer places since when calling a function, arguments don't have type signatures.
Similarly, this:
if (x && y && myFunctionThatReturnsBool() && caseNumber3 && (15 > 12 || 2 < 3)) {
...
}Is also less readable than this:
if (x && y && myFunctionThatReturnsBool()
&& caseNumber3
&& (15 > 12 || 2 < 3)) {
...
}This example is from the WindowAC module. Here is the current code:
// These initializations are done every iteration
InletNode = state.dataWindowAC->WindAC(WindACNum).AirInNode;
OutsideAirNode = state.dataWindowAC->WindAC(WindACNum).OutsideAirNode;
AirRelNode = state.dataWindowAC->WindAC(WindACNum).AirReliefNode;
// Set the inlet node mass flow rate
if (GetCurrentScheduleValue(state, state.dataWindowAC->WindAC(WindACNum).SchedPtr) <= 0.0 ||
(GetCurrentScheduleValue(state, state.dataWindowAC->WindAC(WindACNum).FanAvailSchedPtr) <= 0.0 && !ZoneCompTurnFansOn) ||
ZoneCompTurnFansOff) {
state.dataWindowAC->WindAC(WindACNum).PartLoadFrac = 0.0;
state.dataLoopNodes->Node(InletNode).MassFlowRate = 0.0;
state.dataLoopNodes->Node(InletNode).MassFlowRateMaxAvail = 0.0;
state.dataLoopNodes->Node(InletNode).MassFlowRateMinAvail = 0.0;
state.dataLoopNodes->Node(OutsideAirNode).MassFlowRate = 0.0;
state.dataLoopNodes->Node(OutsideAirNode).MassFlowRateMaxAvail = 0.0;
state.dataLoopNodes->Node(OutsideAirNode).MassFlowRateMinAvail = 0.0;
state.dataLoopNodes->Node(AirRelNode).MassFlowRate = 0.0;
state.dataLoopNodes->Node(AirRelNode).MassFlowRateMaxAvail = 0.0;
state.dataLoopNodes->Node(AirRelNode).MassFlowRateMinAvail = 0.0;
} else {
state.dataWindowAC->WindAC(WindACNum).PartLoadFrac = 1.0;
state.dataLoopNodes->Node(InletNode).MassFlowRate = state.dataWindowAC->WindAC(WindACNum).MaxAirMassFlow;
state.dataLoopNodes->Node(InletNode).MassFlowRateMaxAvail = state.dataLoopNodes->Node(InletNode).MassFlowRate;
state.dataLoopNodes->Node(InletNode).MassFlowRateMinAvail = state.dataLoopNodes->Node(InletNode).MassFlowRate;
state.dataLoopNodes->Node(OutsideAirNode).MassFlowRate = state.dataWindowAC->WindAC(WindACNum).OutAirMassFlow;
state.dataLoopNodes->Node(OutsideAirNode).MassFlowRateMaxAvail = state.dataWindowAC->WindAC(WindACNum).OutAirMassFlow;
state.dataLoopNodes->Node(OutsideAirNode).MassFlowRateMinAvail = 0.0;
state.dataLoopNodes->Node(AirRelNode).MassFlowRate = state.dataWindowAC->WindAC(WindACNum).OutAirMassFlow;
state.dataLoopNodes->Node(AirRelNode).MassFlowRateMaxAvail = state.dataWindowAC->WindAC(WindACNum).OutAirMassFlow;
state.dataLoopNodes->Node(AirRelNode).MassFlowRateMinAvail = 0.0;
}Here is a modified version with local reference variables. This is also a good way to reduce some of the verbosity associated with the state parameter. For what it's worth, this is one of the few uses of both references and the auto keyword that I consider "not terrible" programming.
// These initializations are done every iteration
auto &windowAC(state.dataWindowAC->WindAC(WinACNum));
auto &inletAirNode(state.dataLoopNodes->Node(windowAc.AirInNode));
auto &outsideAirNode(state.dataLoopNodes->Node(windowAc.OutsideAirNode));
auto &reliefAirNode(state.dataLoopNodes->Node(windowAC.AirReliefNode));
// Set the inlet node mass flow rate
if (GetCurrentScheduleValue(state, windowAC.SchedPtr) <= 0.0 ||
(GetCurrentScheduleValue(state, windowAC.FanAvailSchedPtr) <= 0.0 && !ZoneCompTurnFansOn) ||
ZoneCompTurnFansOff) {
windowAC.PartLoadFrac = 0.0;
inletAirNode.MassFlowRate = 0.0;
inletAirNode.MassFlowRateMaxAvail = 0.0;
inletAirNode.MassFlowRateMinAvail = 0.0;
outsizeAirNode.MassFlowRate = 0.0;
outsideAirNode.MassFlowRateMaxAvail = 0.0;
outsideAirNode.MassFlowRateMinAvail = 0.0;
reliefAirNode.MassFlowRate = 0.0;
reliefAirNode.MassFlowRateMaxAvail = 0.0;
reliefAirNode.MassFlowRateMinAvail = 0.0;
} else {
windowAC.PartLoadFrac = 1.0;
inletAirNode.MassFlowRate = windowAC.MaxAirMassFlow;
inletAirNode.MassFlowRateMaxAvail = inletAirNode.MassFlowRate;
inletAirNode.MassFlowRateMinAvail = inletAirNode.MassFlowRate;
outsideAirNode.MassFlowRate = windowAC.OutAirMassFlow;
outsideAirNode.MassFlowRateMaxAvail = windowAC.OutAirMassFlow;
outsideAirNode.MassFlowRateMinAvail = 0.0;
reliefAirNode.MassFlowRate = windowAC.OutAirMassFlow;
reliefAirNode.MassFlowRateMaxAvail = windowAC.OutAirMassFlow;
reliefAirNode.MassFlowRateMinAvail = 0.0;
}This is especially handy for "nested" array lookups of which EnergyPlus has many.