This article discusses a coding practice that I discovered while an undergrad a while back, maybe 1994. After doing some code reviews recently, I am reminded that this particular practice still needs more publicity. When writing code, a developer typically likes to worry about edge cases first, then handles the normal case. This practice has some benefits: the cases you worry about most get handled early. And, this practice creates maintenance nightmares. This practice has a some detrimental effects:
- Code readability is reduced
- Precious cycles are wasted on the uncommon cases
- Code complexity frequently increases
People remember and read positives easily. They have more issues with negatives. Consider the following C# code:
1 FileStream fs = null;
2 const string filename = "someFile.txt";
3 if (!File.Exists(filename))
4 {
5 fs = new FileStream(filename, FileMode.CreateNew, FileAccess.ReadWrite);
6 }
7 else
8 {
9 File.Delete(filename);
10 fs = new FileStream(filename, FileMode.CreateNew, FileAccess.ReadWrite);
11 }
Consider what happened when a developer wrote the code above. They thought, "OK, if this file does not exist, then I can create it. However, if the file does exist, I want to make sure I start with a clean slate, so I’ll delete the file, then create it." The code above is confusing. The thoughts are equally confusing. While I wish this code were an anomaly, it isn’t. This is something I see on a regular basis: in support forums, in code that didn’t have time for a proper review, and elsewhere. Consider what would have happened if the developer had chosen to use positive logic instead of negative:
1 FileStream fs = null;
2 const string filename = "someFile.txt";
3 if (File.Exists(filename))
4 {
5 File.Delete(filename);
6 }
7 fs = new FileStream(filename, FileMode.CreateNew, FileAccess.ReadWrite);
Here, the code is more readable and the intent is clearer: if the file exists, delete it. Then, open the file for read/write. Note that the code is now smaller and maintenance is easier too.
Here is another example, this one a PowerShell script from TechNet magazine. Here, we have an example where readability is diminished thanks to negative logic. Line 15 just bugs me. Read the code:
1. function Get-WmiInventory {
2. param (
3. $wmiclass = "Win32_OperatingSystem"
4. )
5. PROCESS {
6. $ErrorActionPreference = "SilentlyContinue"
7. $computer = $_
8. trap {
9. $computer | out-file c:errors.txt -append
10. set-variable skip ($true) -scope 1
11. continue
12. }
13. $skip = $false
14. $wmi = Get-WmiObject -class $wmiclass -computer $computer -ea stop
15. if (-not $skip) {
16. foreach ($obj in $wmi) {
17. $obj | Add-Member NoteProperty ComputerName $computer
18. write $obj
19. }
20. }
21. }
22. }
OK: If I should not skip then I should do work. With a little better naming, we could make this all more readable. How about this instead:
1. function Get-WmiInventory {
2. param (
3. $wmiclass = "Win32_OperatingSystem"
4. )
5. PROCESS {
6. $ErrorActionPreference = "SilentlyContinue"
7. $computer = $_
8. trap {
9. $computer | out-file c:errors.txt -append
10. set-variable shouldWriteComputerInfo ($false) -scope 1
11. continue
12. }
13. $shouldWriteComputerInfo = $true
14. $wmi = Get-WmiObject -class $wmiclass -computer $computer -ea stop
15. if ($shouldWriteComputerInfo) {
16. foreach ($obj in $wmi) {
17. $obj | Add-Member NoteProperty ComputerName $computer
18. write $obj
19. }
20. }
21. }
22. }
Here, readability goes up because I can now see exactly why we DO go into the loop instead of seeing why we do NOT.
If you write negative logic, do a pass after writing things to fix up the code. Do that pass immediately. I frequently write code like:
if (not X){
&#
160; Do A
}
else {
Do B
}
I immediately turn around and rewrite this code as:
if (X){
Do B
}
else {
Do A
}
I have found that I can read this kind of code long after writing it and remember what I meant. I have difficulty doing the same thing with negative logic paths.
Karmic Code Monkey 