Here's a simple common-sense tweak you can incorporate into your own programming style:
The Problem
What I often come across in other people's code is complicated if statements that check a number of things without giving hint about the actual intention if the statement. You have to read the whole series of comparisons to find out what the original programmer actually wanted to do.
This is an example from my own code, intentionally converted to this unreadable style:
(rectangle.X >= 0) &&
(rectangle.Y >= 0) &&
(rectangle.Right < testedPackingAreaWidth) &&
(rectangle.Bottom < testedPackingAreaHeight)
) {
// [...]
}
The Solution
There's a simple way around such statements: Create a temporary
boolean whose name denotes exactly what the checks are trying
to verify. This could be something like
fileIsAccessible, isContained or,
in my case, isInsidePackingArea:
(rectangle.X >= 0) &&
(rectangle.Y >= 0) &&
(rectangle.Right < testedPackingAreaWidth) &&
(rectangle.Bottom < testedPackingAreaHeight);
if(isInsidePackingArea) {
// [...]
}
Since the boolean is the first thing you see when reading this piece of code, you'll be told right away what the following series of comparisons is trying to achieve. You can easily skip the checks and concentrate on the actual logic.
Whilst debugging, this style also allows you to see the result of the comparisons. Granted, in the "bad" style, you can judge the outcome by looking which branch of your if statement the program runs into, but this is just more convenient.
Regarding performance, there should be no worries. Within the boundaries of a function, modern compilers can basically do anything, including writing a totally new function as long as its effects and results are the same. Trust it to optimize away the temporary variable to the exact same code that would be generated for the complicated if statement.
Additional Tips
The human brain doesn't understand negations as effectively as
it understands direct statements. Therefore, it's better to
choose non-negated expressions in your code. For example, instead of
isNotInsidePackingArea, use
isOutsidePackingArea. Or (a favorite) do
not use isNotWritingProhibited, but use
isWritingAllowed :)
It's also often useful to turn ifs inside-out. Instead of moving an entire function into an if statement, change the if statement to leave the function when the conditions aren't fulfilled:
(rectangle.X < 0) ||
(rectangle.Y < 0) ||
(rectangle.Right >= testedPackingAreaWidth) ||
(rectangle.Bottom >= testedPackingAreaHeight);
if(leavesPackingArea)
return false;
// [...]
This safes indentation and someone reading the code can immediately see what happens when the condition is not met and tick off that code path instead of having to scroll all the way down to the end of the if statement in order to see whether anything is happening or an else branch exists.
Post new comment