Representative Line: Without a Parent

Rob M caught a ticket for a bug in a C# application. Specifically, when the user picked an item off a menu, that item wouldn't get highlighted, thus defeating the purpose of the menu. Strangely, the code hadn't been touched since its first commit, back in 2015.

var sortedParentChildItems = matchedMenuItems.OrderBy(x => x.ParentID ?? x.ParentID).ThenBy(x => x.ParentID);

Somehow, this particular line made it through code review, which was notable because management was extremely proud about how "rigorously enforced" their code review process was. The "rigorous enforcement" took the form of someone filling out forms and running a meeting that hopefully contained more programmers than managers. It created a lot of hours on the time-sheet marked "code review", so it must be rigorous.

In this case, this line has several steps of silliness. The ?? is the null coalescing operator- if the left-hand operand is null, use the right one. So, we'll sort by the ParentID of a menu item, unless it's null, in which case we sort by the ParentID. Once they're sorted by ParentID, we'll sort within that list, by ParentID. If it's null this time, enh, whatever, we don't care.

Arguably, the root cause of this bug was a null value snuck into ParentID somewhere, and that broke the sort order. Realistically, the root cause was a process which was more focused on having a process than doing a useful code review.

[Advertisement] Ensure your software is built only once and then deployed consistently across environments, by packaging your applications and components. Learn how today!