> His code is just… ugly… to the point that you nearly detect a physical odor from it.
Unit tests don't magically bring quality code with them. But it does tend to make it easier to focus on encapsulation.
> "How can we overcome years of inertia, a nasty legacy codebase, ...
Yeah -- this is a real challenge. BTW Bill is not the problem so much as the codebase. If Bill refuses to consider unit tests, that's a problem that I think is solveable.
> Simply walking over to Bill ... "You have a tendency to write enormous methods...
Nothing would make any designer more defensive than saying "you do [bad thing]". Bill would probably confess that he, like everyone else, has contributed a few lines here and there to MegaObject::mega_func(). Your challenge is that Bill is apparently less motivated than you are to address the problem. Focus on the code itself.
"Bill, you know mega_func()?"
"OMG yeah that is a nightmare to debug."
"Well, I've been working on unit tests. It took a while but I found a way to create a unit test to cover MegaObject. Now I can test MegaObject without deploying to the target first."
"Hmm..."
"Newbie Fred took this test for his first feature, a one-liner fix in mega_func()."
"Oh yeah?"
"Yeah, and he was able to find and fix issues with his implementation without going to the lab. And then he refactored mega_func() by adding some encapsulation to those middle 70 lines. We only need that bit in FROB mode anyways."
In my experience, this usually ends with Bill continuing to do what he's always done and you & Fred trying to continue refactoring and adding tests but getting frustrated because more untested mega code keeps getting added so your target keeps expanding. If you're lucky, you'll reach critical mass and be able to start enforcing coverage or lint checks at the build step (at which point you can tell Bill "oh yeah you just need to add test coverage and it will pass" when he comes to you trying to figure out why is build failed), but more often than not you won't be able to keep up and you'll just get burnt out & cranky.
While I absolutely agree with "talk about the code itself" rather than beating around the bush in some manufactured discussion, why not just address the issue head on when is proper to do so, in CR, and build a culture where code critique is both acceptable and clearly ONLY a critique of the code at hand? I like to think that I could highlight mega_func in a cr to our team and go "extremely difficult to ingest this long a function as a human or modularly test it. Refactor would be welcome" and they would say "makes sense" and swap it in the same way as if someone pointed at one of my DB schemas and went "that's going to cause you problems because X and Y and I'm not sure what you were smoking when you decided on your constraints."
We're professionals here but we're not perfect, and the faster we come to a shared understanding that this is fine, and it's part of professionalism to have each other's backs, the faster I've found my teams able to broach these issues without insult.
Unit tests don't magically bring quality code with them. But it does tend to make it easier to focus on encapsulation.
> "How can we overcome years of inertia, a nasty legacy codebase, ...
Yeah -- this is a real challenge. BTW Bill is not the problem so much as the codebase. If Bill refuses to consider unit tests, that's a problem that I think is solveable.
> Simply walking over to Bill ... "You have a tendency to write enormous methods...
Nothing would make any designer more defensive than saying "you do [bad thing]". Bill would probably confess that he, like everyone else, has contributed a few lines here and there to MegaObject::mega_func(). Your challenge is that Bill is apparently less motivated than you are to address the problem. Focus on the code itself.
"Bill, you know mega_func()?"
"OMG yeah that is a nightmare to debug."
"Well, I've been working on unit tests. It took a while but I found a way to create a unit test to cover MegaObject. Now I can test MegaObject without deploying to the target first."
"Hmm..."
"Newbie Fred took this test for his first feature, a one-liner fix in mega_func()."
"Oh yeah?"
"Yeah, and he was able to find and fix issues with his implementation without going to the lab. And then he refactored mega_func() by adding some encapsulation to those middle 70 lines. We only need that bit in FROB mode anyways."