CodeSOD: Nothing But Garbage
Janell found herself on a project where most of her team were offshore developers she only interacted with via email, and a tech lead who had not ever programmed on the .NET Framework, but did some VB6 back in the day", and thus knew VB".
The team dynamic rapidly became a scenario where the tech lead issued an edict, the offshore team blindly applied it, and then Janell was left staring at the code wondering how to move forward with this.
These decrees weren't all bad. For example: Avoid repeated code by re-factoring into methods," isn't the worst advice. It's not complete advice- there's a lot of gotchas in that- but it fits on a bumper-sticker and generally leads to better code.
There were other rules that... well:
To improve the performance of the garbage collector, all variables must be set to nothing (null) at the end of each method.
Any time someone says something like to improve the performance of the garbage collector," you know you're probably in for a bad time. This is no exception. Now, in old versions of VB, there's a lot of stuff" about whether or not you need to do this. It was considered a standard practice in a lot of places, though the real WTF was clearly VB in this case.
In .NET, this is absolutely unnecessary, and there are much better approaches if you need to do some sort of cleanup, like implementing the IDisposable interface. But, since this was the rule, the offshore team followed the rule. And, since we have repeated code, the offshore team followed that rule too.
Thus:
Public Sub SetToNothing(ByVal object As Object) object = NothingEnd Sub
If there is a platonic ideal of bad code, this is very close to that. This method attempts to solve a non-problem, regarding garbage collection. It also attempts to be DRY", by replacing one repeated line with... a different repeated line. And, most important: it doesn't do what it claims.
The key here is that the parameter is ByVal. The copy of the reference in this method is set to nothing, but the original in the calling code is left unchanged in this example.
Oh, but remember when I said, they were attempting to be DRY"? I lied. I'll let Janell explain:
[Advertisement] Keep the plebs out of prod. Restrict NuGet feed privileges with ProGet. Learn more.I found this gem in one of the developers classes. It turned out that he had copy and pasted it into every class he had worked on for good measure.