How to effectively work with teammates whose fixes to bugs cause more bugs?

Question

Everyone creates bugs, including me and my teammates. When bugs are pointed out to them, they're friendly and try to fix the bug. But their fix is 'wrong' and just creates a more subtle bug. Usually this takes the form of them thinking that the bug is an edge case, and they put some special check in.

Sometimes they will add an extra boolean parameter to something that gets passed around. Repeat this several times over and there's 5 booleans getting passed around.

The most irritating is when they put things in completely the wrong place because they don't know how to modify some generic thing to handle their use case. So they just avoid calling the generic code entirely and write lots of their own stuff.

This creates absolute spaghetti code, and they end up duplicating a lot of code to handle what they perceive as special cases.

I'm a junior developer, the same as them, and we have no-one experienced on our team. It's all juniors. To make matters significantly worse, everyone always just merges into master. Usually the reason I find bugs is that I look at a recent commit to master, and I can see in a few seconds that they've introduced a bug.

What can I do? Showing them the bug, pointing out their fix isn't right until they get it right under my guidance.

It feels patronising, I'm not really superior to my peers, all of us are new. Maybe I have picked things up faster than them and I know what they're doing is wrong.
Is time consuming
Sometimes they make such a mess that I feel like I have to tear it all down and do it better. Which obviously is delicate.

It makes me want to just fix the bug myself as soon as I see it, rather than report it to them. But I know this is wrong. Both because it's insulting and also because it makes more work for me.

This isn't a software engineering problem, although it appears in the context of creating software. We don't know you or your colleagues, your company, your culture. How do you think we should come up with a solution to this workplace problem? If you don't have senior developer who can coach your colleagues, you may need to step in and provide guidance, possibly in the form of pair programming which is probably the least intrusive way to nudge them towards better solutions. — Hans-Martin Mosner, Jun 13 '20 at 22:07
OK, I accept the rebuke. Though I don't understand how it is any more of a workplace problem than say, https://softwareengineering.stackexchange.com/questions/154733/my-boss-decided-to-add-a-person-to-blame-field-to-every-bug-report-how-can-i or https://softwareengineering.stackexchange.com/questions/162709/five-new-junior-developers-and-lots-of-complex-tasks-whats-now?rq=1 — Jovash, Jun 13 '20 at 22:17
Those answers are pretty old, so might not reflect current reality here, I don't know. This site has changed focus several times over the years, meanwhile other sites have come about. — Erik Eidt, Jun 13 '20 at 22:56
It seems to me to be an appropriate question of development method: how to handle developer peers of differing competence on a shared project. — Steve, Jun 13 '20 at 22:57
This would be appropriate for https://workplace.stackexchange.com, if that helps. — Greg Burghardt, Jun 13 '20 at 23:52
I'm going to argue against closing this question because there are solutions to this issue that fall under the umbrella of self organizing teams. — candied_orange, Jun 14 '20 at 00:03
@Jovash your comment and the answers already given show that the question has a significant SE aspect where I mainly saw the workplace/interpersonal aspect, so feel free to ignore my comment :-) — Hans-Martin Mosner, Jun 14 '20 at 04:47
I'd suggest hiring a consult to give a seminar on basic software development strategies. — historystamp, Jun 14 '20 at 22:12
You might also want to try pair programming (not the whole time, but, say, 2-3 hours a day, few days a week) - have a discussion on that as a team; this will give you the chance to learn things from each other (and make no mistake, it is going to be a two-way street), and sort of bring up the base level of the "skill landscape" for the whole team. It will also let you work on communication skills and other aspects of being a team. — Filip Milovanović, Jun 14 '20 at 23:44
@Jovash If you yourself are also a junior developer one thing I would consider is if you really want to work at a company where you don't have any opportunity to learn from experienced programmers. Sure you can learn about things like good development practices, devops and so on yourself too, but it really helps to have people who can mentor you and lead by example. — Voo, Jun 15 '20 at 10:30
A thing to consider is the root cause of those "hard to solve" bugs, "edge cases" and the like... Perhaps the design of the code could be improved so that code blocks are less complicated and easier to test? I have had to maintain code where every "fix" felt like dancing on a minefield. Of course, it is up to you and your team to asses if this aspect can be improved. — SJuan76, Jun 15 '20 at 11:05
@Hans-MartinMosner - oh this is absolutelly a software engineering issue. Sloppy, un-reviewed code that doesn't follow best practices is not something for HR to fix. — Davor Ždralo, Jun 15 '20 at 17:25

Arseni Mourzenko · Accepted Answer · 2020-06-14T07:51:35.713

65

Test your code

Have an actual regression testing, so that when a bug is introduced, the programmer finds it right away.

This doesn't mean that you should just write the actual tests. Instead, it means that you need to work on the culture of the team to understand that without regression testing, the project is doomed.

I wrote tests, my teammates then change the tests so that they pass while being incorrect, at one point someone actually disabled the tests.

This, clearly, is a sign that the culture is all wrong. It might be that the culture is wrong inside your team only, or it may be that it's wrong at the management level as well—for instance, someone may have disabled the tests because he was told to do so by the management, after complaining that he can't urgently deploy a bugfix to production because the tests are red.

In all cases, there is work to do to have a solid build pipeline, and to have a team where everybody understands that a change cannot be deployed to production without testing code for regressions.

Do code reviews

By doing systematic code reviews, you can eventually get rid of:

This creates absolute spaghetti code, and they end up duplicating a lot of code to handle what they perceive as special cases.

No code should go to production untested and unreviewed, when written by a junior programmer.

Don't hire junior programmers if you don't know how to work with them

Imagine that you're an owner of a hospital, and you decide to hire only interns. No doctors with twenty years of experience. Nothing like that. They are costly, after all, and any intern should be able to do exactly the same thing, right?

Junior programmers will build crap. Not because they are bad persons, but because:

They don't have enough experience.
They qualify themselves as programmers, i.e. they write code. This is not the most important part of the project, and a project which is composed of programmers only will fail.
In order to improve, they need help from more experienced software engineers.

Your management decided that the project you are working on is not important for the company. They actually decided it, because they actually hired junior programmers to save money. What they didn't know is that not only the project will fail, but it will cost a lot of money, much more compared to if they hired a few experienced software engineers.

From there, you have to decide what to do. Either you can convince your management to stop being stupid. Or you can't. If you can't, you may either continue working on a doomed project, while trying to keep your head low (when the management will find that the project is failing, they will search for culprits, and if you take initiatives, you will be the culprit), or you ask to be moved to a project which is more important for the company.

Working with junior programmers requires skills and practice. If you have no persons with those skills, hiring junior programmers is as stupid as hiring only the interns in a hospital and hoping everything will be just fine.

edited Jun 14 '20 at 07:51

answered Jun 13 '20 at 22:59

Arseni Mourzenko

135,365

4

Please let me clarify that I am also a junior programmer, it's all junior programmers on my team. Point 3. is perhaps good advice to my company as I do feel there is some mismanagement of us juniors. I did ask that we do code reviews, but people proceeded to merge straight into master anyway, and code reviews would only bring the errors to my eyes more immediately, not help in actually effectively resolving them. I wrote tests, my teammates then change the tests so that they pass while being incorrect, at one point someone actually disabled the tests. – Jovash Jun 13 '20 at 23:03
3
will not work. The code the people create does eventually work, but is unmaintainable.

Oliver Meyer

Jun 14 '20 at 07:20

13

@OliverMeyer: the three points in my answer are cumulative. It's not “You can do tests or code reviews,” but “You have to test your code and you have to do code reviews.” – Arseni Mourzenko Jun 14 '20 at 07:53

Thank you for your updated answer. I have been considering leaving my job due to disatisfaction anyway, so there's little to lose if I try to make them realise the issues with their approach. And definitely noted re the keeping my head low, though I think that is too late for me. I'm recognised by my company as the reason things are working, I fear a corollary to them will be I am the reason it is not working. – Jovash Jun 14 '20 at 11:51

"they actually hired junior programmers to save money" - the question is can they afford to hire a senior, and can another business afford to lose one? I don't just mean at the current salary, but after the raise another employer may give to retain or recapture the lost senior. Most bosses have a sense of "business self-esteem", and although low self-esteem may prevent them hiring better even when it appears rational in a first-round analysis, the function of it is to stop them getting dragged into what a dynamic (multi-round) analysis would show is ruinous competition for senior devs. (1/2) – Steve Jun 14 '20 at 14:05

That is, to the boss, it's not an arithmetic question that if he can afford 10 juniors then he can instead afford 5 seniors on twice the wage. Rather, the fact that the seniors are on double the normal wage inherently shows they are scarce, whereas the juniors on low wages must be abundant. If he poaches 5 seniors from another employer by upping their wage, other employers who need those seniors will then be looking to poach from him by upping the wage again (perhaps dramatically), and turnover will surge. With 10 juniors on low wages, this will not happen. (2/2) – Steve Jun 14 '20 at 14:12

11

"code reviews would only bring the errors to my eyes more immediately, not help in actually effectively resolving them."

The thing about code reviews is that when they are in a system for code reviews, IE any of the big ones like Github or Bitbucket, you can reject the code. Then the person that broke it has to fix it before you allow them to merge it. If they merge it anyway, that's when you go to management. Even senior devs benefit from code review, why would a junior assume it's too good for them?

– n_b Jun 14 '20 at 18:46

1

This suggests OP to ask management to close project and fire himself to stop losing money. – Basilevs Jun 15 '20 at 05:55

1

I must say that the "take initiatives" => "your fault" connection you make isn't necessarily the case. Any manager with even a modicum of competence would see anyone who took efforts to save a doomed project in a good light. Even better would be the position he'd be in if his initiatives did save the project. – ScottishTapWater Jun 15 '20 at 06:40

1

@Persistence the OP will be the best judge, but this does not sound like a situation where there are managers with a modicum of competence. It sounds like a situation where managers are more interested in what helps them than what helps the organisation, and such managers seldom pass up a good scapegoat. – James_pic Jun 15 '20 at 12:46

For this specific case, I think you should move code reviews to the top of the list. And not just "reject bad commits" type of reviews, but reviews where the teams gets together and goes over the changes and discusses them. That will give the OP the opportunity to show the errors and demonstrate how/why tests should be used. Also, OP is showing senior tendencies and should take this as an opportunity to become a de facto leader by example. And maybe some git patterns could be added to the list. – Colin Young Jun 15 '20 at 18:56

candied_orange · Answer 2 · 2020-06-14T09:03:56.623

Start by inviting people to pick apart your work.

You don't need management to create a command structure to fix this problem. You need teammates that trust you.

You build trust by showing you're not just some power hungry jerk that's looking to get out of work by spending all day approving other peoples work.

Get others to help you improve your work and you show people how to accept input from others. It makes them feel less threatened when they need to do it.

This is part of being a leader. It's not something management can bless you with. You either do this or you don't. You emerge as a leader when people are coming to you with their problems.

Doing this will not grant you authority. What you gain is influence. And influence is enough to fix every problem you've mentioned.

score 13 · Answer 3 · answered Jun 14 '20 at 00:11

You have two problems. One is that your team allows merges without code review. That isn’t just unprofessional, that is utterly stupid. I don’t even do that for private use code that I create on my own. You need to call upon a team leader or management to enforce mandatory code reviews or decide as a team that that is what you want. I can tell you that if anyone votes against code reviews (developers or management) I wouldn’t allow them anywhere near my workplace.

The second problem is that your teammates don’t create good bug fixes. That should improve with time. Making them fix incorrect bug fixes is educational and improves quality of future fixes. It seems that creating a quick fix that doesn’t work doesn’t have any negative consequence for the developer, that is also a management problem.

There needs to be an understanding that the action for a bug report must not be to do enough work to fix the bug report, but the actual problem needs fixing. Good luck.

score 5 · Answer 4 · answered Jun 14 '20 at 18:39

I disagree that this is purely a workforce matter. True, your workplace has issues, but there are a few things you can do that will start helping with the problem by addressing the way you handle code.

Do code review

It sounds like you aren't doing code review, and it does not matter at all what the excuses are, because code review will not only help people spot bugs before they go into code, everyone will also become more familiar with the code. If you do not have a system set up for code review, consider migrating your code to a Bitbucket or GitHub repo, and then require people to use those tools to create pull requests and review code. If you cannot use either of those for some reason (probably someone in management?), you should probably quit, because it's not going to get any better and you aren't going to learn any skills that will help you in your career. If your team is all juniors, I'm sorry, but you probably aren't working on anything important enough that a fear over "stolen code" or "service outages" would make any difference. Both Bitbucket and Github have incredibly cheap options for small teams (and you can have free private repos on Bitbucket at the time of this writing), so it would be foolish not to use them.

Add pre-merge hooks

Add pre-merge hooks to your code so that it can't get merged without a code review and approval. The services above have this capability, use it and enforce it. Not only can you ensure that everyone's code has been vetted, you can also add hooks that stop people from accidentally blowing away master by messing up their commit.

Write tests.

Nobody likes writing tests, but you are all juniors, so it's the blind leading the blind when it comes to code quality- anything helps. Tests can even be made requirements before merging, depending on your system.

Those three things are not open to debate (aside from which service you choose). Someone on the team may have their head in the clouds and may not think that you need those systems in place, that "we can do it without ", and whoever that person is, please have them go tell Google, Facebook, Microsoft or literally any other software company how their way of doing things will work in contrast to tried an true ways of doing things. Just because your company is small and inexperienced is no excuse not to use the tools that everyone else uses to maintain code quality. A simple Google search as to how those companies, or any big company, does source control will show you that they aren't being cavalier about it.

Static analysis.

No matter what your programming language, there is something out there that will help analyze your code and point out errors or enforce coding standards. That way it's not even a person pointing out the issue, so you can reject pull requests simply by saying "the machine complains."

As a final suggestion, one that almost no manager wants to hear, try pair programming. Fight for a few hours a week to begin with, where your team sits down with eachother and pair programs. If they argue that nobody does pair programming, Nordstrom's mobile teams pair program, and create a product that was responsible for about 1/3 of the company's revenue (at least, was in 2018)- that's over $100 million USD.

Oh, and if you aren't using Git or Mercurial for source control, why aren't you using git or mercurial for your source control?

Thank you for your response. To answer your somewhat implicit question re source control, we use git with bitbucket. I really like the idea of static analysis and pre merge hooks, things which help the person writing the code realise things are wrong rather than that them leaving it to someone else to realise would be a great help. — Jovash, Jun 14 '20 at 19:33
Excellent! I'm glad you don't have to fight the battle for using git or bitbucket. Bitbucket has a primer on how to set up the hooks- https://confluence.atlassian.com/bitbucket/suggest-or-require-checks-before-a-merge-856691474.html If you all have admin access, then someone could disable these checks easily, though — n_b, Jun 14 '20 at 19:38