@cartman82
Thank you! This thread is exactly what I was looking for.
I'm suffering under code review from an EA who writes code that looks like a stackoverflow question. I've worked with some great EAs who can't write code. The difference is that they know it and stick to what they're good at it. It's so nice when everyone does what they're good at.
Every time we need to put messages in a queue and read them, he copies a "template" consisting of eight (!) projects full of really bad code, most of which is ignored, but we can't delete it. There is no composition. It's all layer upon layer upon layer of inheritance, spread out across multiple NuGet packages with no debug symbols.
I could write pages about what's wrong with this code. One extra-special gem are the constructors in the "template" classes we have to use. There's one constructor that creates all sorts of concrete dependencies. There is no dependency injection. Then there's a separate empty constructor with a comment:
// For unit testing.
All of the unit tests call that constructor. He mentioned it - "You know, when you create a separate constructor for unit testing." Like I would know. I don't know. I tried to explain the problem, which is that your unit tests don't test the same code that your application actually runs. He dismissed this, calling it "a pattern we follow everywhere." I guess if it's a pattern that makes it better.
The rest of the code is contorted around that "pattern." Methods that should be private are public so that the unit tests can call them. The class maintains unnecessary state - which has already resulted in serious defects - so that the code path is observable to unit tests. In a normal reality you would inject abstract dependencies, mocking them, and test the code by observing its interaction with the mocks. (And this would be really simple because each class would only have at most a handful of simple dependencies.)
I could go on and on about the outright hostility and temper tantrums that occur when I try to delete dead code or change the names of classes and methods to vaguely reflect what they actually do, since they're at best meaningless and at worst misleading. It's like one of those neighborhoods where all the streets are named after birds and six of them are named "Heron." It's almost calculated to avoid clarity.
Anyway, my contract is almost up. It's snowing outside. Not my circus, not my monkeys.