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.

What triggered the code changes? A bug report from QA (the feature has yet to reach production)
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.

Tests can be written. The important thing here is that the architecture chosen was not good enough. This should have been caught before any code was written. — Thorbjørn Ravn Andersen, Jul 29 '23 at 15:08

score 17 · Accepted Answer · answered Jul 07 '14 at 16:06

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.

score 9 · Answer 2 · answered Jul 07 '14 at 16:37

9

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.

answered Jul 07 '14 at 16:37

Telastyn

36,240
9
87
127

"think they are incompetent and that they're so bad .. that you don't even want to work with them". This is what I feared he would think. I would've liked to try work with him on this, but it wasn't an option at the time. The work is now done and in the codebase. Is it still possible to start a conversation purely about the problem? – Andy Hunt Jul 07 '14 at 20:36
@AndyBursh - it's probably better than just letting him find out about it himself. – Telastyn Jul 07 '14 at 20:44
@Chris, Telastyn Would you agree that then that allowing him to find out for himself, and being prepared to participate in a code review, is the best approach? – Andy Hunt Jul 07 '14 at 20:48
@AndyBursh: No, it's better to address it directly, but as Telastyn said you should talk about the code not about the person. Try to phrase it like you made his good work even better and ask if he agrees or has more suggestions. – Jul 07 '14 at 20:53
@Chris That sounds like a good, positive solution. Thanks! – Andy Hunt Jul 07 '14 at 20:59
@AndyBursh - you should definitely not just leave the code there. You should "suggest a better solution", which would be easier to do before burning down their hard work. – Telastyn Jul 07 '14 at 22:52
1

+1 : Going behind someone's back (senior or not) and undoing their work is on the border of unprofessional – Vector Jul 20 '14 at 08:42

score 3 · Answer 3 · answered Jul 07 '14 at 19:11

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.

At the the time of writing, the changes have been made silently and pushed in to the main branch; as a matter of circumstance (his not being present) rather than choice. Is it still possible at this point to start a conversation in a positive manner? — Andy Hunt, Jul 07 '14 at 20:42
Absolutely, if the relationship overall is a reasonably good one. "Hey boss, we made some changes to some of your code to help us go forward while you were gone. We checked them, but can we get you to review them with us to see if we've created issues?" If the boss likes going forward this should come across OK.
But if the boss is defensive/difficult, you've lost him/her at "made changes to your code", and if the boss is a bad coder you still have a problem getting to agreement on the changes. That's a different question. — John G, Jul 15 '14 at 20:46

Handling low quality work from a senior

3 Answers3

Linked