The case of the unbalanced brackets by Mark Seemann
A code mystery.
One of my clients was kind enough to let me look at some of their legacy code. As I was struggling to understand how it worked, I encountered something that looked like this:
ApplyDueAmountG89.Calculate(postState.PartialMebershipsBAT.Where( d => (d.Data.Choicetype == GarplyChoicetype.AtoC || retirablePartialMembershipNr.Contains(d.Data.PartialMembershipNr)).ToList(), ApplyDueAmountG89.Situation.Depreciation, ApplyDueAmountG89.RecordType.Primo);
For the record, this isn't the actual code that my client gave me. I wouldn't post someone else's code without their permission. It is, however, a faithful imitation of the original code. What's wrong with it?
I'll wait.
Brackets #
Count the brackets. There's a missing closing bracket.
Yet, the code compiles. How?
Legacy code isn't humane code. There's a multitude of ways in which code can be obscure. This article describes one of them.
When brackets are nested and far apart, it's hard for the brain to parse and balance them. Yet, on closer inspection the brackets seem unbalanced.
Show whitespace #
Ever since I started programming in F#, I've turned on the Visual Studio feature that shows whitespace. F# does, after all, use significant whitespace (AKA the Off-side rule), and it helps to be able to detect if a tab character has slipped in among the spaces.
Visual Studio shows whitespace with pale blue dots and arrows. When that feature is turned on (Ctrl + e, s), the above code example looks different:
ApplyDueAmountG89.Calculate(postState.PartialMebershipsBAT.Where( ····d·=>·(d.Data.Choicetype·==·GarplyChoicetype.AtoC·||··············································· ············retirablePartialMembershipNr.Contains(d.Data.PartialMembershipNr)).ToList(), ············ApplyDueAmountG89.Situation.Depreciation, ············ApplyDueAmountG89.RecordType.Primo);
Notice the space characters that seem to run off to the right of the || operator. What's at the end of those spaces?
Yes, you guessed it: another Boolean expression, including the missing closing bracket:
d.Data.Choicetype == GarplyChoicetype.BtoC) &&
If you delete all those redundant spaces, this is the actual code:
ApplyDueAmountG89.Calculate(postState.PartialMebershipsBAT.Where( d => (d.Data.Choicetype == GarplyChoicetype.AtoC || d.Data.Choicetype == GarplyChoicetype.BtoC) && retirablePartialMembershipNr.Contains(d.Data.PartialMembershipNr)).ToList(), ApplyDueAmountG89.Situation.Depreciation, ApplyDueAmountG89.RecordType.Primo);
Imagine troubleshooting code like that, and not realising that there's another Boolean expression so far right that even a large screen doesn't show it. In the actual legacy code where I found this example, the extra Boolean expression started at column 209.
Conclusion #
Hiding significant code so far right that it's effectively invisible seems positively evil, but I don't think anyone did it deliberately. Rather, my guess is that someone performed a search-and-replace through the code base, and that this automatic change somehow removed a newline character.
In any case, keeping an eye on the line width of code could prevent something like this from happening. Stay within 80 characters.
