At a previous job, I became "The Code Review Guy". It was a big company, with a lot of bureaucracy. They were transitioning from VB6 to VB.NET and didn't trust developers to adapt to this new world, so they positioned code reviews as a "gateway" and the reviewers were guards, tasked with ensuring that any code going past met the standards.

That was already a pretty bad, and pretty hostile approach. Then some code would get submitted which didn't just violate the standards, but was barely comprehensible nonsense which followed no coherent convention and couldn't be tested let alone debugged. But it was mission critical and had a scheduled release date already, so the code review process had to let it pass. "Just make some notes, and they'll fix it in a future release," was the attitude. You can imagine how much of that code got fixed.

In any case, one of our standards was that developers should use a StringBuilder object, not string concatenation. Concatenation produces many, many intermediate strings, which can cause performance and garbage collection issues, while a StringBuilder avoids that.

This standard was commonly violated.

Which brings us to this representative line of C#, which Adeline found in a customer's code-base. I can assume that they had a similar standard: use a StringBuilder. Whoever wrote this followed those instructions to the letter:

StringBuilder body = new StringBuilder();
body.AppendLine("Batch " + name + " batch header sheet " + string.Format("{0:C}", Convert.ToDouble(headerAMT) / 100) +
                " and Check total " + String.Format("{0:C}", Convert.ToDouble(totAMT) / 100) + " do not balance!\r\r");

Adeline adds: "I wish I could say this code wasn't indicative of the whole."

[Advertisement] Otter - Provision your servers automatically without ever needing to log-in to a command prompt. Get started today!