What to do when Your Colleague Creates Spaghetti Code
Editorial Note: I originally wrote this post for the NDepend blog. You can check out the original here, at their site. While you’re there, take a look around and give NDepend a try.
I write for a number of different outfits and earn my living consulting around software and IT. Because of the intersection of these three concerns — writing, offering advice professionally, and software — I field a lot of requests for advice on how to do the right technical thing without everyone around you shooting holes in it. Consider an example.
“How can we get started with unit testing?”
Considered as a technical question alone, this invites a fairly obtuse response. “First, write a unit test. Second, run the unit test.” Obviously, that’s not really what anyone is asking when they ask this question.
Instead, they want to know something orders of magnitude more complex. “How can we overcome years of inertia, a nasty legacy codebase, Bill, who has been around for 40 years and hates everything new, and the fact that management doesn’t want to pay us to write ‘extra’ code… and then start unit testing?” Oh, yeah, that. Well, that’s complicated.
I frequently hear such apparently-innocuous-but-actually-complex questions about code quality. “This idiot on my team is writing mountains of the most unmaintainable garbage imaginable — what should I do?”
Usually, this sort of question comes to me from people at client sites. And, accordingly, I have to balance the answer that makes the most sense for the individual with the one that keeps the client’s best interests in mind. The client pays my bill, and I have a charter to lower the cost of ownership and development of their code.
But when I’m in writer mode, and only the asker’s interests matter, I’l give a subtly different answer. Today, I’d like to offer advice on what you should do in this situation.
A Note of Disclaimer
To be clear, the question “what should I do” does not imply that you and your organization move in lockstep when it comes to your interests. Your organization may want the cleanest, most maintainable code imaginable, and, theoretically, it would demand that you fight the good fight.
But your organization might also throw “Angry Bill, the unit test hater” at you and tell you that he’s the senior developer, like it or not. Suddenly, dying on the “clean code” hill becomes a less clearcut choice.
I thus offer the following disclaimer. My advice on what you should do may not always drive the best outcome for your company.
Make a Data-Based Case
Having read my disclaimer, there you sit, convinced that Bill is slowly poisoning each codebase that he touches. His vertical and horizontal spacing infuriates you, his public method semantics offend your sensibilities. His code is just… ugly… to the point that you nearly detect a physical odor from it. You might even think he’s doing it to spite you.
As much as you hate it, are you sure it’s bad? Are you editorializing at all when you describe it as spaghetti? I mean, how do you know? Have you charted the entry and exit points and graphed out the dependencies? Or do you just not like it because he does things much differently than you?
It’s not that I doubt you. It’s that you can bet most people you make your case to will doubt you, especially Bill. If you don’t arm yourself with some data about the code, you’ll look spiteful when you state your case. And if you don’t back the data with supporting references that aid your case in declaring it a problem, you’ll do little better.
Do some research before going any further and make sure you can justify your accusations.
Ask Yourself, “Just Who Do I Think I Am?”
Ask this question because you can be sure someone else will. Sure, you’ve built a bulletproof, case, leveraging static analysis, expert opinions, and industry trends. That’s all fine and good.
But you’re about to accuse Bill — Bill! — of writing spaghetti code. Bill, who started with your company when they had carrier pigeons instead of emails, and who singlehandedly built the original version of the software with nothing more than COBOL, baling wire and duct tape. Just who do you think you are, anyway?
Seriously, this matters. You need to consider your role. If your team looks to you as technical leadership and a mentor, than going to a new team member and saying, “your methods are pretty complex — try to keep this thing called cyclomatic complexity under 3” presents no problem. Everyone, newbie included, expects you to do this in your role. But if you’re the newbie and the junior developer saying the same thing to the mentor, it’s going to get ugly.
I can hear your indignant protest that “roles shouldn’t matter when there’s an empirical demonstration of rightness!” Honestly, I agree with you. And I imagine HR will want to hear all about it at your exit interview.
Arguing from a position of relative inexperience exacts a political toll from you — even when you’re right. Have these battles at your own peril, and check out the term pyrrhic victory to understand what happens even when you win one.
Offer Alternatives and Outcomes
Let’s assume that you’ve built a decent factual case and accurately sized up the organizational capital at your disposal. You have enough of a case to proceed and enough credibility that people will listen to your case.
Don’t actually proceed until you can offer some kind of solution. Simply walking over to Bill and saying, “I’ve been doing a lot of research and here’s a lengthy treatise on why your code is terrible,” will only demoralize him. Instead, you need to prepare by having alternatives ready to offer. “You have a tendency to write enormous methods, but you can actually take a lot of those local variables and extract classes with them as fields.” Now, you’re offering a plan of action.
But go beyond that as well, and complete your case. Once you’ve shown Bill the better way, make sure you can then prove (or something close) better results will follow. Get creative. Maybe pull someone over and ask them how long it takes them to understand the old method and the new, broken-up classes. Whatever you have to do, help the “aha” moment along by offering benefit.
Humility, Diplomacy, and Leading By Example
You’ve built a case, stated it, and demonstrated it alongside value offered. Don’t let that go to your head.
You don’t want to take the attitude that “my way is right and yours is wrong.” Rather, you want to let the data do the talking and decouple you and Bill, as humans, from the code that you have written. You’ve read a lot and learned from smart folks that better outcomes occur this way. You’re not an expert, but you’ve had good results yourself, if he wants to take a look. Here, look, you’ll show him if he’s interested.
Do your best to adopt something of a patient mentor stance, but without implying that one of you is better or more experienced… even if you clearly are better and more experienced. The subject shouldn’t be you and Bill, but the relative merits of various techniques. You’re just a couple of peers discussing them.
A Closing Philosophical Note
I touched on this in the beginning of the post, but I’d like to restate it here. “How can I get Bill to stop writing Spaghetti code” is only superficially a technical problem. In reality, it presents a human problem.
You can have the right of it without making any difference. Or, you can browbeat Bill and entrench him further. You can send him angry emails, undo his commits, “reject” his stuff at code review, and work against him. That will demoralize him and torpedo everyone’s progress.
Simply put, you can’t alter Bill’s behavior without a whole lot of dealing with Bill. So, whatever route you take, understand that your question is one of persuasion and not technical correctness.