(Note: The developer in question does not work for my company.)
I'm looking at some code in one of the products I'm responsible for, and I just came across this.
// ReSharper disable once RedundantAssignment
Tuple<List<SomeChartData>, string> objReportdata = null;
Let's review the WTFs:
- The developer didn't understand ReSharper's admonition that the "= null" is completely useless, so he disabled the warning.
- The Tuple is actually the return value of the method. It doesn't even need to be declared as a variable; it can simply be returned to the caller.
- Why are we using a Tuple in the first place?
- Oh; the string part of the Tuple is the title of the chart. This is a code smell, but I haven't analyzed it deeply enough yet to figure out how to clean it up.
- Wait, so why a Tuple? Why don't we simply have a first-class data transfer object that has everything one needs for a chart?
- OK, so even with a Tuple, why are we declaring it with a List instead of an IEnumerable or ICollection? What business is it of the calling method what concrete object we use to return chart data?
- Despite repeated directions to read our sodding code standards document, he named the variable—which is useless, remember—with a forbidden Hungarian prefix and used what I'm going to call for the moment "humping camel case." I don't know what that camel is really doing, but I'm pretty sure "data" should be capitalized.
The entire method of 40 lines had too many WTFs in it so I simply had ReSharper re-write it for me. It's now 20 lines, avoids the second of two doubly-nested foreach loops, and uses named constants instead of magic numbers. And it still stinks.
</rant>