27

I have a coworker who has been practically the sole contributor on a service for a number of years. For the most part, he does an above-average job. It works fairly reliably, it is thoroughly tested, and is relatively easy to read on a line-by-line level.

The software also has a lot of room for improvement. The details don't really matter to this question, but the flaws are completely uncontroversial to others I have discussed them with. Things like parsing a highly-nested grammar with regular expressions. He also takes decoupling to the extreme, to the detriment of other equally-important design principles.

These problems have built up over time, to the point where making changes now takes significantly more time than anticipated, and my coworker really needs help to keep the backlog from growing.

The problem is when anyone else tries to help out, he takes the changes personally, gets very defensive about his code, has difficulty admitting there is anything wrong with his design, and blames problems on user error or on code other people wrote. In other words, he seems completely blind to flaws in his code.

He is kind and pleasant about it, but also very stubborn, and will stand his ground for days, despite multiple people disagreeing with him. If we approach him before we make the changes, it's even worse, because he has a hard time visualizing the result. We don't technically need his permission to make improvements, but it is the courteous thing to do.

It is a draining process for everyone involved, and frankly, I don't want to spend the energy anymore. How can I approach making improvements to his code without having to argue (politely) for days to get it merged?

  • 6
    Who's in charge? Who sets priorities, allocates resources, and makes decisions about how reasonable a delivery timeframe or maintenance effort is? – dwizum Aug 9 at 13:28
  • 2
    related (possibly a duplicate): How can I deal with a difficult developer that is holding back the project? – gnat Aug 9 at 13:57
  • 3
    Is it possible to break out a chunk of the service code so it's now someone else's responsibility? So the original maintainer can just treat it as a black box while other devs handle any changes to that component? – DaveG Aug 9 at 14:16
  • "Reason for termination: Does not play well with others." – EvilSnack Aug 10 at 22:46
  • To state the obvious, the company owns the code not the coworker. Propose a set of changes to your manager, get the OK from your manager and fix it. – user25792 Aug 13 at 14:10
24

How can I approach making improvements to his code without having to argue (politely) for days to get it merged?

Based on the text of this question, you seem like a very reasonable person and have done the necessary things to try and be inclusive and considerate of the other developer when introducing new techniques.

At this point, I think you should bring this to the attention of your manager. You should not have to debate every single commit, especially when the approach is clearly better.

You have already tried working with the individual directly, and it has proven exhausting. Let your manager deal with this going forward.

  • 9
    This could be a good answer, but it depends on the manager. It runs the risk of making OP look whiny if the manager is not technical enough to understand the need to make the code changes. If the manager is technical, then he can be convinced (or not, if he doesn't think the changes are a good idea) that there is a problem. At the very least it will give him a heads-up if there is blowback. And getting his permission to make the changes transfers future problems to the manager, which is, I think, where they belong. – Francine DeGrood Taylor Aug 9 at 15:51
  • 2
    The manager does not need to be technical for this case. What need to be understood, both by the manager and the developer, is that code you've written on the job is not yours, is the company's. – Spidey Aug 10 at 6:43
  • @FrancineDeGroodTaylor if the manager is unable/unwilling to see the advantages of the move, so be it; it would be the manager's responsability and not the OP's. What the OP cannot do is overstep his functions and assume the manager's position just because he feel s like it. And a situtation that risks creating a personal conflict with the team is certainly the manager's responsability. The problem here is not technical, but personal. – SJuan76 Aug 10 at 8:39
  • @FrancineDeGroodTaylor: Ideally, this would be framed in terms that a non-technical manager can understand, e.g. "[Module X] has a lot of technical debt, so much so that [feature Y] took [Z] extra [days/weeks/months] to implement because of it. [Person A] and [Person B] agree with me that we should spend some time refactoring, but [Person C] seems to disagree. When I sent them [bug/pull request Q], they got offended, which was not my intent. How do you suggest we move forward?" – Kevin Aug 10 at 19:00
  • I'm with Francine on this. Our non-technical manager takes EVERY issue of this nature that is brought to him, calls a meeting of all programmers, tries in vain to explain the issue to us, then asks for consensus. As a result of this, we all resent him and one another. – id est laborum Aug 11 at 2:01
10

How can I approach making improvements to his code without having to argue (politely) for days to get it merged?

Don't approach him anymore when making improvements to the code. You have already attempted this on multiple occasions and the results have not been desirable.

At this point, since you stated that his permission is not needed, simply go ahead and work with your coworkers to improve the code when necessary. Don't attempt to re-write the whole program, only modify and improve the code necessary for your specific task.

I wouldn't mention anything to the coworker until after the changes are complete and thoroughly tested and you can demonstrate that the new code is an actual improvement. This is very important, the code you and your other coworkers write has to be better than the original otherwise you not only upset the coworker but you lose credibility.

  • 9
    Bus-factor. Bus-factor. Bus-factor. – user44108 Aug 9 at 13:38
  • 5
    The problem with this approach is that you can get into a cycle of passive aggressive behavior where the OP improves the code the way the OP wants it, the original maintainer then improves the code back to the way he wants it to be. – DaveG Aug 9 at 13:54
  • 1
    @DaveG, you are definitely correct that this cycle could start. The way to make sure that doesn't happen is to have the concept of code reviews. If the original maintainer starts making unrequested changes, possibly causing the original bugs again, this should be caught before they are deployed. At that point, the manager can be again notified about the changes, further adding to the cause of the OP. An employee that makes changes that aren't requested can sometimes be against policy, even if they have the best of intentions. At worst, it could be considered wasting time or malicious. – computercarguy Aug 9 at 21:46
  • Another way to forestall this might be to add unit tests covering what you fixed. If passing the test suite is a requirement for code to be accepted, then your changes can't simply be backed out. (And if the test suite can be triggered automatically on check-in, then you're not directly involved.) In a previous job, the penalty for breaking the build was to buy cakes for the rest of the team! It was a fun but effective approach. – gidds Aug 10 at 18:16
  • You are misreading the question: it is not that the asker cannot change the code, but that their changed are not accepted - the specifically relevant part of the question is "without having to argue (politely) for days to get it merged?" You imagine that it is reversions that would not get through code review, actually it is the asker's "improvements" that are being rejected in review. – Chris Stratton Aug 12 at 6:07
6

You should consider issues of (a) preference and (b) performance separately. The couple of issues you describe (using regex for parsing, or putting a priority on decoupling) may be more in the camp of composition preferences and not major performance limiters.

In both cases though, you should focus on the value at stake (how many hours or headaches or dollars would be saved by a change) and avoid criticizing existing implementations, regardless of how bad they may be.

Also realize that tech debt is undesirable, but it happens in every team and fixing it can be a substantial cost. Sometimes it really is better to leave things as-is even when you have to constantly create work-arounds.


(a) For issues of different preferences, it's good to describe the alternatives you think of, but your colleague likely gets the final say on how the package is composed. You should also make clear that your suggestions are your own preference, and not an objectively better way to compose code. Even where highly-opinionated standards for composition have been created (e.g., PEP-8 & Black), these remain opinions and may not be a good fit for the problems your organization works on. Debating preferences as peers will be a far friendlier approach than criticizing the preference of your colleague.


(b) For issues of software and/or team performance, it's also good to suggest alternatives, but take some reasonable steps to ensure your suggestions are productive:

  1. What is the quantified performance impact? If an alternative to parsing with regex runs 100x faster, how many hours of compute would be saved over the course of a year?
  2. What kind of effort will reworking take? Multiply your estimate by 3. And now that you know you're going to multiply by 3, you might be biased low, so multiply by 3 again.
  3. How relevant is the performance of the section of code to the needs of your customers? Do your customers care about performance (whether in processing latency or cost)?
  4. How does your performance improvement idea compare to other improvement ideas? There are likely a lot of ideas in the "parking lot" for your team - is this performance improvement substantial enough to become the priority?

With those questions answered, you could propose the idea to your team.

3

Assuming you're both at the same seniority: If a guy told me that I'm changing his code the wrong way (and I know for sure I'm not), I'd tell him to fix it his way. Then when he claims he doesn't have the time I'd politely tell him to stfu. You're stretching the problem out by treating him like a delicate flower. Or option two: Just stop helping him 100%. Get your co-workers to do the same. Then his problem of not being able to fix his code in a timely fashion will definitely rise to management. When management asks why you aren't team players, you all point fingers at him. Honestly this guy should have been fired years ago. Your manager is asleep for sure. Even the most hands off manager should be aware of how long it's taking to fix this guys code.

If he out ranks you that's another story. Time to look for another job, perhaps.

  • Pointing fingers in this way has a high potential to backfire. The problem dev can point fingers right back. Unless the OP and co-worker talk to the manager first about the problem, there's no way to quit working with the problem. If they aren't following the managers orders to work with this person, they could be disciplined or even fired. The OP needs to work with the manager rather than working unilaterally. – computercarguy Aug 9 at 21:49
  • In my experience in situation slightly similar where a guy was not pulling his weight, it never even got to the level of pointing fingers because at some point that guy realizes that it's his problem and not your problem. And he's the one who's going to be in trouble if the ball gets dropped. I agree with your implied point, it's everybody's problem on a team. But when someone isn't being a team member that's different. You can't reward people who aren't team players. It only gets worse. If it comes to a crisis the problem will definitely get solved. – HenryM Aug 9 at 23:07
  • 2
    @computercarguy ...I mean if the situation is not a true crisis. At any rate there must be accountability. If one guy is a problem that has to be addressed. – HenryM Aug 9 at 23:28
2

First of all

  • It is your opinion that his design and code is flawed.
  • It is his opinion it isn't.

Opinions make trench wars and unhappy colleagues. You need to make it measurable. Tools exist that can analyse code and extract things like depth of hierarchy, test coverage, etc. Find a tool that can measure what you think is important and make the team agree on what should be done and use the tool to see you get there.

Perhaps you can analyze why bug-fixing currently takes so long, so you can help him improve where he actually wants to improve. You might be wrong in your assumptions.

  • This is effectively circular. Your "measurements" themselves only derive from opinions. That a tool reports something might be a fact, that the report is relevant or appropriate to prioritize over some other goal is a matter of opinion. You can select, configure, or build a tool to confirm any opinion you hold. The actual issue is the disagreement over goals. – Chris Stratton Aug 12 at 6:02
  • @ChrisStratton Did you read the whole thing? "Make the team agree on what should be done". In other words the team agrees on common goals and how to measure if you are reaching them. – Thorbjørn Ravn Andersen Aug 12 at 10:39
  • You give no suggestions for how to reach such an agreement. That is the original problem, which is why your answer is circular. – Chris Stratton Aug 12 at 11:31
  • @ChrisStratton Team appears larger than the two persons presented here. Again the solution is to make it measurable and to decide for a goal on what is wanted as a team and then use a guide to get there. If that is not possible for team, then the team is dysfunctional and a reorganization may be necessary. The antagonist may have to be removed from "his" code as it isn't. – Thorbjørn Ravn Andersen Aug 12 at 12:41
  • "Making it measurable" an irrelevant distraction from the immediate issue when there is no agreement over what to measure. If your only proposal for solving a fundamental disagreement of goals is to remove the original author, things will probably look great for a month or two as people start to get what they want but the codebase will quickly become an unmaintainable and inconsistent mess with no one curating change with respect to a consistent architecture, and most of the knowledge about why past change requests were refused lost and taking a long time to painfully rebuild. – Chris Stratton Aug 12 at 13:24
0

To play devil's advocate, has anybody that disagrees with your colleague attempted to understand their point of view?

They have worked solo on this project for years (did anyone express interest before now?) and now, despite having a reliable, heavily tested product with easy to read source code, people want to undo their code because they think their code is better.

If your colleague is unable to visualise the result, then you're failing to communicate the idea within the context of the product. Remember, this is something that they've been thinking about for several years, they know it inside and out.

The solution to this issue is a simple 4 step method:

  1. Suggest (make your suggestion);
  2. Listen and acknowledge (listen to their response);
  3. Negotiate (find a compromise);
  4. Action (enact the compromise).

To be perfectly honest though, if I'm reading the situation right from the question, you probably won't have to worry about the colleague for much longer as they are probably already planning on changing employer as the environment would be feeling very much "anti them".

-1

Get the work you believe needs to be done added to your todo list, spreadsheet, tfs, issues, jira, whatever. Get your manager to sign off on it being done. Then do it.

I wouldn’t recommend a meeting (formal or not) to discuss what should be done, and I wouldn’t add all of the work at once. Just pick something that you think can be improved and get it put on the list. Then do it. Rinse, lather, repeat.

  • It's not that the asker is unable to work on the code or make changes, it is that their changes are not accepted into the codebase: as indicated by their ending the question with "without having to argue (politely) for days to get it merged?" There's a real disagreement over goals there: doing an end run around that disagreement and the original author who disagrees won't change that, there has to be a proper decision about what the goals actually are. Otherwise what you basically have is a fork, and that's not normally viable in an organization. – Chris Stratton Aug 12 at 6:12
  • You make design decisions in proper design meetings, not in task scheduling ones. – Chris Stratton Aug 12 at 6:15
  • @ChrisStratton: I was suggesting avoiding meetings entirely. The question says they don’t need the “authors” approval, but are instead seeking it out of politeness. My suggestion is basically to drop the politeness, and get the “author” used to changes being done without his approval or even input. But do so gradually. – jmoreno Aug 13 at 11:04
  • Sound software projects don't work that way, with anyone free to make whatever change. And indeed the question specifically mentions the obstacle of needing to get their code accepted for merge. – Chris Stratton Aug 13 at 13:42

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.