Disagreement with project lead on coding standards [closed]

https://softwareengineering.stackexchange.com/questions/331406

28-12-2020
|

Question

So I'm working on a new project along with my project lead for the past 1 year.

Initially we had our own sub-projects that resided in separate git repos, I had little interaction with his code, so the code smell didn't bother me. Some 6-months later I began to maintain and add features to his code, as I was taking a bigger role in the project.

Now that I'm the lead developer for both sub-projects (team about to grow; he's still above me), these things bother me and I wanted to take care of them, but was refused:

No curly braces, capitalized functions, mixed quotes usage (double and single w/ hidden logic), not using ===, huge classes with huge functions. Bottom line, could be better.
Reliance on PHP's option to turn off notices/warnings. Code is full of unchecked usages of variables and array keys. Variables get defined inside of ifs.

Arguments to above 2 issues:

Not wanting to enforce a coding style on people.
Regarded as a language feature, which lends itself to shorter/more efficient code.

I believe that some rules need to be had and code should be defensive. I offered to use PHPStorm's default settings for the formatting, use curly braces and a community-accepted naming convention.

I want to align both projects to use the same guidelines, as they are inseparable.

Am I in the wrong? Do I impose my personal preferences?

Solution

If you can make a strong case for why yours is better (and what major problems may come of using his), then you are not imposing personal preference, I think, but instead trying to set a project up for success.

If he can make a stronger case why his is better and what kind of problems it will prevent that yours can't, then he's got you trumped.

Decent upper management should be able to see the merit in that, but those are the points they will (and should) care about: not whether it's easier for developers or "don't want to force everyone to work like a robot" (which is a ridiculous point, but don't use it as a pain point to upper management). They (should) care about the ongoing stability of the project.

Per my comment above:

If he is the project lead, that seems to imply that it's basically his call.

That was my thought. If you want to change the standard but he's not budging on it, either a) figure out how to get above him, b) go somewhere else to work, or c) try for what little things you might be able to do without irritating him too much.

Getting above him is only going to work if you make a strong case for why there should be better standards.

OTHER TIPS

Basically:

you don't like his coding style. That's your right.
he finds it OK like it is and finds spending days/weeks/more just adjusting the style is a waste of time. That's his right.

Put yourself in his shoes. What are the benefits of your endeavor, besides of costing the company lots of money (your salary)? Is he always so nitpicking? Doesn't he see there are more important stuff to do right now?

Basically, you have to find some kind of compromise you can live with, or your relationship will become sour. It depends also a lot on how you communicate with him. Put your ego aside and try to be helpful ...because lastly you are asking him things you would like, not something that he has interest in. In other words, you are asking him a favor.

It also often depends on how you ask. Examples:

What you want:

making the code prettier

What not to say:

your code sucks the way it is

What to say:

I would really appreciate it if I could make both projects consistent, just the basics, and it'll just take me a day.

What you want:

no more unchecked warnings

What not to say:

your code sucks the way it is

What to say:

I know a quick way of getting rid of this and that warning. Then, by re-enabling them, we could detect more quickly if something might be broken in the future.

And lastly:

I know it's not a priority. So I can gladly do it once high priority stuff is off the table first.

Or, if that doesn't work out or you have a bad relationship with him, consider leaving. If you can't sort out things now, it's unlikely to get better in the future ...and put your ego aside.

Side note

I think it's more common about senior people to be more lenient. They know the code will be trashed in a decade or two anyway (because technology changes, APIs too, teams, business partners, requirements, global decisions, whatever...). They have less ego and focus on having it working rather than being perfect. Although I tend to perfection myself, I can't blame them.

This is probably going to be controversial but...

We talked and agreed to disagree and escalate it to higher management

Never. Ever. Do. This. EVER. Every time you do this, it sets all of us back a decade. This is how this will play out:

Senior manager 1: "We have been asked to deal with this issue blah,blah,blah, something about coding standards and wasting time."
Senior manager 2: "And they're asking us to resolve their nerd turf wars because?"
Senior manager 3: "Because they're whiny babies who can't communicate and don't care/understand what business value is?*"
Senior manager 2: "That must be it. Who's up for golf?"

Then comes you and your boss's performance review time:

"[senior manager to your boss]: I'm sorry but there's some concern that you are not able to manage your direct reports and you may not be following best practices (even though I have no idea what you do except type some mumbo jumbo that makes the software worky)"

"[senior manager to you]: I'm sorry but there's some concern that you don't relate well to your peers and have trouble following the directions of your immediate supervisor."

And this is why we're mostly underpaid compared to the value we create.**

You need to either A) get over it or B) move on to a shop that has the standards tuned a little closer to your liking. FWIW, I'd do B (and hopefully move on to a language that doesn't permit such atrocities). But never ever escalate a technical dispute to the suits unless it involves something illegal or potentially catastrophic (gaping security hole). Merely annoying doesn't cut it***.

* For the sake of people who will read this and misunderstand, I'm not saying that the OP and his boss are like this (I realize that code quality/legibility has a direct impact on the business's bottom line), I'm saying that they will be perceived like this by non-technical management.

** For people who think my portrait of upper management as being clueless a-holes is unrealistic, understand that the managers care (ideally) about managing people the way we care about managing code. They may be clueless about technical matters, but they know people, and that's all the more reason that bringing them a dispute that A) they are probably not qualified to resolve and B) the two of you arguably should have been able to resolve without help makes you look bad.

*** Yes I realize that technical debt can pile up to the point of sinking the business. I just don't think curly braces will save you (that being said, I never omit the braces and strongly prefer others don't either).

IMHO, you're facing a 'it's working' developer, not one that would care for the next going after them. His arguments are just worthless.

All that you say about his code is just raw laziness on his part. Having projects following the same coding standard is about rigor. You're not robots; you're engineers; you're supposed to be rigorous.

You could point out by some examples that you're having a hard time following read his code, and that is a huge loss of productivity for you, but I have no real idea how to bring that to him.

But I will likely answer to you something is the tone :

It's working, no point to change

My advice : if he's really blunt and doesn't want to head anything, let it go. Wait for errors/bugs/evolution to happen in his project. When it comes, just fix and rewrite the part of code modified/added to a proper way; don't go to him to say anything about it. He won't impose on you his style of coding, since you're not a robot anyway....

When you work on a project, you have to set priorities.

Priority #1 is to get along with your co-workers. Priority #1 is followed by a huge gap. Then come priorities for things like code that is working, testable, tested, documented, maintainable etc. Then comes another huge gap, and then come coding standards.

And changing existing code to conform to new coding standards is really low on the priority list.

PS. "Micro-optimisation" vs "super-fast computers": Totally wrong argument. The argument against micro-optimisation is not that computers are fast, the argument is that time wasted on micro-optimisations means you have no time going after real savings.

PS. If it only takes you one day to make changes that make the code look better to you, but upset your co-worker and make you an enemy, then you wasted one day on something that is just counter productive.

PHP is not the most elite group in our profession. Actually you are criticizing his probably hard-won software system. Your professional standard is higher as his one.

Then if you have no leeway to improve the code under your responsibilities, there last just one solution: move on.

I do not know about the current PHP market at your place, but you might first diversify somewhat. Learn some hype language too, or a niche like C#.

You might not get such a nice job, but you will come further, professionally. So have patience, and do some self-studying. Safe money. Then ask for some leeway in your projects, or take some job offer.

I find it hard to believe that #1 is his real reason for not wanting to change the coding standards. If you own the codebase, you should be able to enforce whatever coding standards you (and the other developers) agree to. If you achieve consensus with the other developers (assuming the lead is no longer doing any development work) then there's no reason for him to truly care, except for this one:

Is fixing the style of the code base the best use of your time right now?

I'm sure you have deliverables. How much time will it cost to go through and improve style everywhere in the other code base? What if you introduce bugs by fixing style incorrectly? From Uncle Bob:

Of course bad code can be cleaned up. But it’s very expensive. As code rots, the modules insinuate themselves into each other, creating lots of hidden and tangled dependencies. Finding and breaking old dependencies is a long and arduous task.

Improving code style like this is almost never a good use of time as a standalone sprint item. The way I like to make these sorts of improvements is what I call the "Good Neighbor Policy": don't go around fixing all the style and logic structure you can find, because you probably invest as much time as you want and still will not be done. Instead: whenever you are making a change to one part of the code, fix the style while you are there, and leave it better than you found it. If you are struggling to implement a feature because the code is designed badly, fix the style to unblock yourself rather than beating your head against it.

In this way, each change:

Has a business motivation in addition to the technical perfectionism motivation
Will not be viewed as a large time investment (because it's not!)
Will establish good stylistic habits precisely because you and your team are doing it over time
Will not present a large risk to the code base because you're not changing too much at once

A few months from now you'll look up and notice that the "terrible package" isn't so bad anymore and your boss will see that his team is happier. But because it wasn't a direct confrontation it will already be done, and he won't have anything to complain about because you didn't waste time (in his mind) by adding a big refactoring project to the to-do list. He likely will never tell you that you were right, but that isn't the goal anyway (right?).

Ask these questions about your lead.

Are they used to working alone or with a very small team?
Have they mostly coded at this one shop?
Are they used to making the decisions?
Are they used to "just getting it done"?
Did they write most of the code?

If the answers are "yes", then I'm going to paint a picture of a particular type of lead programmer. If it matches up with what you've experienced, maybe it will help get into their head. If not, ignore this answer.

This is someone who has been there from day one, has spent years at the same job working on the same code base, is used to having their way, and doesn't have much experience with other ways.

They don't consider other people when writing code since it all makes sense to them. Of course it does, they wrote it, or they've spent years understanding it.

They consider coding style to be a personal preference, not a tool to reduce maintenance and bugs. When arguing about coding style they will struggle to hear your arguments because they've probably never thought much about why they do things their way. What they will hear is "I want to do it my way" or "I want to do it the new, fancy, trendy way".

They're set in their ways. Because they have been doing it the same way for so long all their tools and editors and brain micro-configured to exactly their style. Any deviation from this style will conflict with this carefully arranged, but very brittle, way of working. Attempts to change will draw complaints about their editor, tools, the way they like to work, or it being "hard to read". They reject change because they've wrapped themselves up so tight in the status quo they cannot change.

This is someone who has never properly learned software engineering and software architecture. They just sort of bang together whatever works.

You have a people problem, not a technological one.

You're going to have to retrain your lead, or you're going to have to quit.

Going to management is a last resort. Both for the reasons @JaredSmith pointed out and because you will lose. This guy has spent years making money for them. He wrote their company. He's put out in numerous fires. To you he's a cowboy chef making spaghetti. To them he's a hero that built and saved the company.

To retrain you'll have to...

Gain his trust.
Figure out how he thinks.
Address his fears about change.
Make changing easier.
Show how this is better for him.

Take his style seriously and get inside his head. Ask him about it. Why does he do things the way he does? What does he see when he reads it? How does it interact with his tools? How does he move through the code? Knowing all of these things will allow you to understand and address his objections.

Find the objective root of his subjective objections, make them actionable. "It's hard to read" is subjective, and it gives you no information. You can't do anything about that. "I'm color-blind and syntax highlighting doesn't work" is objective, it gives you information, and you can do something about that. I would recommend a book called Getting To Yes for more on that.

Once you get at the root problem, the real problem he's experiencing, see if you can fix it or mitigate it. Then it's not a problem. They'll probably still have emotional issues with changing, but at least they can no longer argue it's an actual problem.

Do it a little bit at a time. This is someone who has been doing it the same way for years. He is used to seeing certain patterns in the code and using them to understand it. Suddenly changing all those patterns will be confusing. As frustrating as it will be to slowly bring them up to speed with known good practice, you have to walk him through it.

Advocate for a standard community style. This eliminates the argument that it's about personal preference and puts the pressure on them to justify why their different style is so much better. If you plan to be hiring, it makes it easier to integrate new hires.

Advocate for automated code style. Make following the correct style a push of a button. Use a tool that starts with a standard style, lets you configure it to your tastes, and can restyle the code with a push of a button. Making it as easy as possible to follow the style removes many arguments about how difficult it will be to follow. They can code however they like, and when they're done they push a button and it follows a style others can read.

Since this person is not in the mindset of thinking about others, you will have to show how these changes benefit them. It can be as simple as "since this is the standard now, you won't have to go through this fight again with the next person you hire". Or it can be "if we have tests you can be more aggressive about changing the code and worry less about changing things". Or "if there are good docs, people won't have to keep bothering you with questions about how the code works". For this to be effective you'll have to know what they want-- some people like to be bothered, it makes them feel important.

It's a long, long road. You'll have to decide whether you have the patience to manage up and retrain your boss. Think of yourself more as their teacher than their frustrated underling, and you might feel better about it.

Am I in the wrong?

I don't know PHP, so I can't make a direct judgement, but I'll assume for the sake of argument that your preferred coding style is "better" than the code you've encountered, since yours is more in line with existing automated tools.

Then you are not wrong to suggest improvements to coding style.

Bottom line, not code I want to work with

You may be wrong to refuse to work with code that doesn't meet your preferred standards, but only to the extent that by working for the company you agreed to work with their code in the first place. Ultimately it's not "wrong" to quit your job if it makes demands of you that aren't to your liking, since that's your right, and you might find a better job as a result.

Do I impose my personal preferences?

No. He's the project lead and you are not. It's his call, although in this case he's "delegated upwards".

He might just as well have decided to delegate downwards to you, as lead developer of the sub-projects, and give you free hand to set the coding standards for those sub-projects and colleagues who work on them in future. But for whatever reasons he feels quite strongly that you shouldn't standardize. Even if he's wrong about coding style you can't expect to claim an authority he hasn't allowed you.

However, since he says "I don't like to enforce coding styles", you can at least write new code in your preferred style (and have done so). Ultimately this might result in opportunities to demonstrate the objective benefits of your style. That would be a good time to have a second go at making your case.

You can also (I think) reasonably ask people who edit files, to do so in the style the file is already written. That allows you to maintain standards in files you wrote. Unfortunately the flip side of this is that you'd have to edit the files he wrote in something similar to his style.

Even assuming that you have a really good test suite, and therefore can refactor in relative safety, there are still (admittedly fairly marginal) reasons not to blast through and re-style entire files. The main one is that it's a nightmare trying to merge, re-order or revert changes made before and after a big structural change. But it may well be that in this particular project that hardly ever happens.

[My manager says he does not] like to enforce coding styles upon people [because] we are not robots.

Ever wonder why we don't just throw the source code away after we compile it and it passes all the tests? Source code is for humans, and it's not just for humans to write, but also to read.

Sooner or later, somebody will have a reason to go back and read the code. Maybe they'll need to change it, maybe to document it, maybe to reuse it. Whatever. It's going to happen, and the code will be a whole lot easier to read and to work with if it's all in a consistent style.

Even a bad style is better than no style at all.

Licensed under: CC-BY-SA with attribution

Not affiliated with softwareengineering.stackexchange