Handling low quality work from a senior

Question

This is a question regarding the software industry, but would apply equally to any industry in which teams contribute work to a final product.

How do I handle having to completely rework the work of my team leader?

Recently, I was undertaking some pair programming at work with a peer (actually my senior developer, but closer to a peer than a senior) in which we decided that the work done by our team leader in the previous weeks would need to be re-done. Briefly: there were absolutely no tests and the methodologies and the approach taken was not maintainable in the long run.

As it happens, he has been on holiday in this time; though I believe we'd have made the same decision were he in the office. How best can I\we handle telling him what we've done? I can defend the technical decisions we made, but I'm not confident that he'll accept the reasons for the genuine technical reasons they are (i.e. the actions taken were not intended as any kind of personal attack or reflection).

---

Update to answer questions in comments.

1. What triggered the code changes? A bug report from QA (the feature has yet to reach production)
2. Has anybody else has reviewed his code? No. There is a code review tool and process in place, but it's rarely used and followed.

Answer 1

Code reviews are the answer. I assume if you're doing paired programming, there is a code review methodology in place as well.

Where I'm currently working, all code gets reviewed before it can be merged into the master codebase. This is the place where issues can be raised and discussed with the developer in a non-confrontational manner.

Raise your issues during this process with your coworker and offer what you think is the better way. If it's a matter of just following coding standards (not a matter of which way is better--just which way is "accepted"), explain that standards are important for ease of maintenance.

If you're not doing code reviews, definitely institute them.

Answer 2

How best can I\we handle telling him what we've done?

Ideally, by not telling him what you've done, but by asking "Wouldn't X be better?" beforehand. Going behind someone's back (senior or not) and undoing their work is on the border of unprofessional. It gives the impression that you think they are incompetent, and that they're so bad (either as a person or professionally) that you don't even want to work with them to make it better.

For software engineers, I would focus on the problem the code is solving. Pretty much all software engineers love to solve problems. By focusing on what the code is trying to solve, you're focusing on that problem, not the problem you have with their code. You're being a good team player and presenting a better solution for that problem. Then you can debate the relative merits versus the problem rather than criticizing their code.

Answer 3

There are many paths, the best one depends on the environment.

Given you have already done the deed, the best way to present it is "Jane and I were using your code, and as we used it we came up with some ideas to improve it. Can we review these with you (e.g., in a code review) and see what you think?" Now the senior is in the authority role, if not the actual expert, and the discussion can be about the technical changes. Hopefully you can have that discussion in a healthy engineering way, and the senior can be pleasantly enlightened.

If there's likely to be bad karma -- e.g., your senior has a history of writing bad code and being defensive about it, and isn't there when you need to decide what to do -- then you have to choose the lesser of evils up front: make the changes silently; don't make the changes at all; or make the changes to allow you to go forward, then trash (and rebaseline) them before merging into the visible code base. Each of these has obvious consequences, but you will be in the best position to guess what they are.