Our Team Lead had been hyping up how important it is to improve the statistics of our codebase. So we have a report that runs and gives you metrics about duplicated code, “code smells”, possible bugs, test coverage etc.
Our Team Lead said that Colin had some great points and would be presenting them in our meeting.
and Colin’s points were
- Can we add rules?
- Can we remove rules?
- Should we document our changes?
Which are all basic questions and not any kind of guidance. Obviously, the answers are
- obviously,
- obviously,
- and the majority of the time, no; that would be overkill
He also did a demo to illustrate what we can include in our documentation. The “code smell” was that there was the same assignment twice like:
somethingEnabled = whatever!=null && somethingElse!=null otherFeatureEnabled = whatever!=null && somethingElse!=null
So the assignment logic is the same. Then he said the solution is to create a new variable. Yes, that is obvious…. but he has some timing tests to illustrate the performance difference of 0.00025 seconds. Since he has proven that this is the correct thing to do, we now must document that as the official solution.
Embarrassing. So cringey.
Then the next day, Colin was talking about his work item and said if he has permission, he would also fix the “code smells” that were declared by the reporting software. So the Team Lead asked him what the “code smells” were and Colin said “unused using statements”.
Since when did developers ever declare/ask for permission for something so trivial? We just did it because it’s obvious. Even Visual Studio tells you to do it. “Unused usings” are shown in a lighter font, and you have an icon in the side prompting you to remove them. It’s part of the Code Cleanup feature too, so you run it and it automatically removes them.
Any developer would just do these refactorings as standard.
There’s a general rule called DRY; Don’t Repeat Yourself that means you should avoid duplicated code. We don’t need to document that any further.
Other code smells like extra blank lines, spelling errors, unused methods; are all trivial. You don’t need to explain them. The reporting software actually has explanations and examples for all the rules anyway.
Stranger Coding Tings 