How can I deal with a coworker who doesn't particularly care about code quality?

Question

I'm a software dev in a small team. I am the most junior member with a couple of years professional experience, but was brought to the (newly formed) team because I had the most hands-on experience with the technology required for the project.

Work has been going well for the past year, but one thing that constantly gets on my nerves is the low-quality code our most senior member commits. I'm personally very strict with code quality and style and always check that all my touched files don't create new issues with our Linter, StyleCop etc. However, he ignores these mostly. He views it as not important and seemingly wants to focus on function rather than form.

Now I don't mind if someone checks in a double blank line or a wrongly positioned brace, but empty catch statements, missing null checks or badly named classes are things that shouldn't go on our main branch, in my opinion. We do not have dedicated code reviews at the moment, and I currently don't see me in a position to push for them.

I normally have no issues discussing this with my peers (we all make errors, myself included), but I have issues discussing this with him. He has been with the company a lot longer and seems kind of sensitive to discussions about his code. In other words, I don't want to piss him off, because I like my current job and I'm likely working with him a lot longer. He's not my superior; we are formally equal.

Should I raise this with my team lead? I'm not sure if I'm just overzealous or too strict here, but considering the product will be very important to our company in the future, I want to strive for quality as early as possible.

Answer 1

Should I raise this with my team lead?

No. The most junior member who has only a couple of years experience should not be complaining to team leads about the code quality of the most senior member.

Let the team lead deal with it - there is absolutely no way that you would be telling them something they don't already know. The reasons that they tolerate it (or perhaps even encourage it as a trade-off for faster time to market) might become more evident over time.

If the culture (as evidenced by the behavior of the senior dev and team lead) doesn't suit your quality preferences, you may need to look elsewhere.

That said, even a junior member can lead by example. Make sure your work is top-notch and the quality is beyond reproach. Make sure you hit all your deadlines. Make sure you help others where needed. Maybe others will notice and want to do the same.

We do not have dedicated code reviews at the moment, and I currently don't see me in a position to push for them.

Later on, when you have established yourself within the group as the local expert in code quality, then you can push for process changes. Maybe you could offer to lead code reviews at that time or help refine and enforce your group's coding standards.

Answer 2

You are developing code for a business, so you need to think like a business person in addition to (in fact, more than) a developer. That level of thinking usually grows as you gain more seniority.

All the coding guidelines, self-documenting code, maintainable code and other shiny stuff mentioned by various experts in books is great to follow, but if you are going to convince people to start following it, you need a more convincing reason than this:

things that shouldn't go on our main branch in my opinion

You need data to back up your claims. You can certainly go to your boss and suggest adopting all the shiny stuff (being a junior notwithstanding), but be prepared to answer the question, "what would be the benefits to the project by doing that?"

Vague statements like "it makes the code more readable/maintainable/testable/etc." (or much worse, "so and so recommends it") doesn't convince anyone to change their tried and tested methods.

Remember that the "poor" code of your seniors has been getting the job done for ages. "Getting job done" is among the most important things in business. Telling people there is a better way of getting job done with only your "opinion" to back it up will not convince them to take the risk.

I would certainly encourage you to push the team to adopt the shiny stuff, but be prepared to put in the effort required to gather the data to support your claims.

As an additional tip, I hope you won't use the tone you have used in this post when you set out to convince your seniors. "Ha ha, you guys suck, I will teach you how it is done" will definitely not work, "I have a suggestion to improve X, and I think these are the reasons why it would benefit us. What do you think?" has a far better chance of getting accepted.

Answer 3

I did experience this in my previous company. It did not end well.

We were two writing the code and the senior colleague was a huge fan of copy and paste programming. ("Remember, there is no problem about that.") The same block of the code was scattered about 50 times around the code base, following the same thought template. I could live with that although I did not like that very much. Null checks through try...catch with empty exception handling were common. After some time I started refactoring that stuff and I was told I am obstructing the work because after the changes he is no longer able to program "from his memory", relying on the older structure. Despite the quality of the fixes and removal of many bugs, I was told "I damaged the code" because in business, no one cares about minor code issues and the only criteria is to get the job done in minimum hours and get satisfied customer. Working with the code led to my unsatisfaction and I was also missing deadlines not able to "copy-paste" fast enough, because I felt I do not want to become such a "fast and straightforward programmer" as I was expected to be. Finally I left and found other company which met my expectations on what I considered to be standards on the field.

I understand there are some "practical" people among programmers. They are often very productive, not paying much attention to details. If you do not like this approach, consider whether to stay in the company or leave.

Answer 4

I do not think you can a lot in the situation. Raising quality issues with your team lead against a senior member can create lot of awkward moments and conversations. (Unless there is a major bug due to which the code will not run at all). Otherwise you may be perceived as someone who has high opinions about himself (or low opinion about others).
I see your concern and I think you should be able to raise these issues but in my experience these things generally do not go well.

My suggestion would be do your best to maintain the code at the quality you desire and let others do what they are doing. Also, from time to time you can causally talk about code quality without pointing fingers at anyone specific. At some point people will notice your efforts and they will have all the revision histories to check. They will recognize you for this. Also, you may be at a relatively senior position in sometime where you can "demand" better code quality from the team.

At this point, I think you should take the extra effort and do best to maintain the code quality yourself and show everyone you are sucker for a clean code!

Answer 5

We do not have dedicated code reviews at the moment, and I currently don't see me in a position to push for them.

Yes, this is what you need (even if you don't think you can).

Anything other than regular code reviews won't resolve those issues.

If your team is large enough, find someone else to have weekly code reviews with. You only need two people to get started. And when a new developer gets hired, have them join in. Code reviews are actually a great way to do onboarding. Of course, this won't solve your immediate problem, but assuming you're able to find another teammate who's willing to participate, it should be easy to get your team lead's permission to start getting the ball running.

Just don't count on that other existing developer to join in unless you already have a large number of team members already participating (which could take years, but ultimately, you have to get started somewhere, otherwise you'll never get started).

And be sure to help run those code reviews as efficiently and as non-judgementally as you can. Even if the team lead offers to help, don't count on him to research the topic as thoroughly as you do and lead those code reviews as efficiently as you can, since this is basically your idea to give it a try.

Again, this does not solve your immediate problem. You wanted to improve his code, not just yours. Unfortunately, new habits like this don't just appear overnight. They need time to develop.

Regarding Masked Man's post:

As to needing data to back up your claims regarding null checks, swallowing errors, bad class names, etc. The book Code Complete has all the data you need to back up such claims. Plus, you could probably track some of those things within your code base by using your current existing analytics package. For instance, you could count the number of times your application fails silently, even when there is a critical piece of information missing.

But I'd suggest you don't waste any of your time doing that. It's simply faster to fix the mistake than trying to keep track of the consequences of that mistake. And since that developer's ego is on the line, data won't change his mind anyway. The same goes with your team lead. Even if he agrees with you in principle, he has to weigh the benefits of good coding practices against the disadvantages of having ongoing arguments with your particular teammate.

Answer 6

Firstly, your coworker might know what he's doing and have reasons for it. For example, if you're doing agile development and showing weekly prototypes to clients, then it might be a perfectly legitimate strategy to implement as much functionality as quickly as possible so that the clients/users can see the design and give their views on it, and then worry about edge cases, error cases, and code quality later when you know that you've got agreement on the requirements.

Secondly, your coworker may be someone who is good at seeing the big picture and bad at following through with the small details. I've been on a number of projects where that kind of person had the vision and set the direction and made a great contribution to the success of the project (though such people often end up not writing any code themselves). There needs to be someone else in that case who is obsessive about the detail. They might (or might not) recognize that description of themselves, especially if you present it to them in a positive way. Try to find out, without causing offence, whether their assessment of their own strengths and weaknesses (and therefore, where they need help from team-mates) coincides with yours.

Thirdly, try to concentrate on the weaknesses that have tangible manifestations, rather than becoming obsessive about "the way you think good code should be written". Avoid subjective criticism (no two programmers agree on all aspects of good style.) And concentrate on the areas of the code that you have a legitimate interest in. Write test cases that fail, and log bugs against them. Add comments or TODOs to the code like "Need to handle I/O failure here".

Answer 7

You say you are strict with code quality, but you are not in charge, so that doesn't matter from the company perspective.

What matters is the company's stance on code quality.

If there is a strict company policy, you can use that to back any discussion with your coworker, but trying to go to your supervisor will create a ton of conflict, so it's best avoided for general style concerns which won't lead to disaster.

If there isn't a strict company policy on this, the only things I'd even consider bringing up (with either your coworker or supervisor) are things that are likely to or have in the past led to the code breaking.

First approach your coworker. Do not tell him the way things need to be (because you should not assume you know better, and there's no faster way to make someone defensive). Ask him his reasoning behind doing it his way and respond with an argument addressing the problems with that, if you can find any (perhaps he has a good reason for doing things the way he does).

If that doesn't work, you can consider bringing this up with your supervisor.

Don't be accusatory or even mention the other team member at all - just bring up any one of the issues and point to one or two examples in your code base, and proceed with the same humble approach mentioned above.

Your supervisor might check or ask you to check who wrote the code (if possible) and this might lead to your supervisor addressing the issue with said coworker. But all of this would be at the discretion of your supervisor. Yes, this might lead to your coworker thinking you "ratted him out", so proceed at your own risk.

---

Coming from an experienced programmer, all of the things you say should ideally be avoided, but none of them are necessarily at the "WE'RE ALL GOING TO DIE!" severity. Ignored exceptions or missing null checks can be disastrous, but they can also sometimes be functionally unnecessary or intentionally omitted. Naming of classes doesn't affect execution at all in any language I know of (reflection notwithstanding) and is thus much less severe than the above concerns which can cause problems at runtime, and it's also generally entirely opinion-based.

So I feel it hurts your argument to put all 3 of those things at roughly the same severity.

But you're certainly free to refuse to work in an environment that doesn't meet your standards and escalate any style concerns as though it were life-threateningly serious.

Answer 8

Institute mandatory code reviews.

You're not in the political position to just demand such a policy shift, but you could propose it to other members of the team and get their buy-in. Once a significant majority of you agree, bring it up with the larger group. Here's a great reason why you should be doing them: https://www.atlassian.com/agile/code-reviews

Our office does this and I love it.

Answer 9

This is a teachable moment

I think it is OK to approach the team lead if you position this as a learning opportunity for you.

Tell the lead you are concerned about code consistency, and want to make sure that you are following the proper standard. Should you be writing what you think is a proper level of code quality? Or should you be doing something more like that senior dev? Perhaps you are overbuilding.

By bringing it up this way, you avoid confrontation yet ensure that the team lead is aware of the issue.

P.S. I suggest you don't press for an immediate answer, and be ready to let it drop.

Answer 10

Although code quality should be important, it needs to be put into the context of management's expectations and the skill level of the team members. Wanting to improve in this area is a good thing, but focus on the problems and not just the symptoms.

Everything you are suggesting is done for a reason. Are users getting errors due to a lack of null checks that require debugging? Every programmer should know the earlier these are caught the easier they are to fix.

After you make the connections between these coding flaws and the problems they are creating for everyone, your team should start looking for ways to fix them. Code reviews or some process to get more than one set of eyes on each line of code is considered a good practice. Where you will run into trouble is if there is no enforcement of what you team thinks is best. If one programmer is allowed to do things his way, none of this is going to work.

At some point, it should be apparent that this person't code is causing more problems than the code created by others. It is up to the team to address problems with sub-standard performers. Hopefully, everyone can come to some consensus on these issues in pursuit of better code.

Answer 11

It doesn't sound like the senior colleague is the problem here. Humans will often take the path of least resistance if given the chance. Your problem is that the process doesn't enforce the quality standards that you deem to be necessary.

Grant me the serenity to accept the things I cannot change, the courage to change the things I can, and the wisdom to know the difference.

The issue you are struggling with is that you don't know whether, as a junior, you're in a position to have any influence over this process. Can you make them change or can't you?

I'd basically try to glean information in a casual conversation (or several) to figure out what they've done in the past and what their objections are to the practices you're suggesting. You'll likely be able to get a sense from this alone of the things which you're likely to be able to affect.

---

Something which you probably do have direct influence over would be asking more senior team members to do informal code-reviews of your code. Remember, you're a junior and so therefore you are useless - having a more senior member of the team check over your work will surely reduce the amount of bugs you introduce and help you to learn*. I think it's unlikely that you couldn't get one person to agree to this.

By doing this, you'll not only have greater confidence in your own code (even if you don't have it in the code of your colleagues) but perhaps you will start to foster an atmosphere where code reviews are seen as a valuable part of the process.

*I'm being facetious.

Answer 12

Nobody likes a tattle teller.

You worry about your own code, and very soon your colleagues will see you as taking ownership of the product.

I'm experiencing a similar dilemma where I'm the senior developer and my peers are junior, many of whom aren't interested in quality.

Time will weed out those who aren't passionate about what they do.

Answer 13

In my experience, code styling is really hard to enforce and always leads to many unfruitful discussions. It's a subjective matter and some people have really strong opinions about it. If you really care about it, you should convince your team to implement an automated build system (e.g.: Jenkins) and add automated source code validation on it. If the code is not linted, the build goes red and won't be accepted.

Let your senior college arge with the machine instead of his junior college.

Answer 14

Should I raise this with my team lead?

Yes.

However, before doing so, read on and make sure you raise the right "this".

Indeed, as the junior member, it's not your place to say "this other developer is writing bad code". However, there is clearly a difference in what you consider quality code you consider acceptable and what code other team members consider acceptable. That is something that would be good to discuss with your team lead. This way, it is much less about faulting someone else and more about finding a good way to continue from here. This may mean that your team lead tells you that you can relax your coding standard. It may mean you end up finding the correct middle ground.

Either way, you feel uncomfortable because of the differing standards people hold their code to, so that should be what you discuss. It shouldn't be whether or not someone else is writing code that is acceptable or not.

We do not have dedicated code reviews at the moment, and I currently don't see me in a position to push for them.

Indeed, you are not in a position to push for code reviews. However, you can suggest using code reviews. It may even be the case that they recognize what advancements can be brought in through a "fresh pair of eyes" and using dedicated code reviews may be exactly such a thing.

Answer 15

Another way to bring this up without getting on anyone's bad side would be to approach the issue obliquely, as one of the team's development process. Identify some past bugs that were observed in the code base that respecting compiler warnings or other tools would have caught before they caused trouble. You can then take that to the team lead and even the whole team, and suggest that the relevant kinds of warnings from those checks be made into errors in your build/test process. No one likes having to re-work code after they're written it and moved on. You can present this as an easy way to make sure you're all getting things right the first time, and avoiding likely bug reports.

Over time, you can mine more classes of bugs that the checkers would have prevented, and get them added to the enforcement list.

As an additional benefit, this approach also avoids putting effort into fixing complaints about the code that consistently turn out not to matter for your particular application. The warnings may not be false positives in the sense of being wrong, but they may be false positives in the sense of not making a difference.

Answer 16

The word for this is root cause analysis. If you can prove that these commits are the source of one or more major bugs, then discuss these commits during the team meeting or defect review meeting as a group to raise the awareness of the negative impact of these techniques. If that is not the case, you can also just email out a snippet to the team and ask someone to provide an explanation, so at least you can improve the documentation.

References

• Improving the software inspection process with patterns (pdf)

• The SEI Personal Software Process: Improve Yourself