16

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:

  1. 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.
  2. 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:

  1. Not wanting to enforce a coding style on people.
  2. 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?

George Kagan
  • 279
  • 2
  • 12
  • Possible duplicate of [How do you make people accept code review?](http://programmers.stackexchange.com/questions/72806/how-do-you-make-people-accept-code-review) – gnat Sep 19 '16 at 12:43
  • 11
    @gnat Coding standard != code review – CodesInChaos Sep 19 '16 at 12:47
  • 1
    @CodesInChaos per my reading the question is about code review. It is inevitable that review is based on some coding standard, formal or ad-hoc – gnat Sep 19 '16 at 12:51
  • 4
    If he is the project lead, that seems to imply that it's basically his call. If you want to convince him, I think you should focus on non-style issues only. For example, requiring that someone write curly braces where they are not required is likely to be seen as nit-picking, so stay away from that issue for the time being. On the other hand, introducing a standard indent policy probably makes sense to everyone (it doesn't matter what the policy is, as long as there is one). – Brandin Sep 19 '16 at 12:52
  • 4
    Curly braces is not nit-picking when you see an if, foreach and another if inside each other :). It's all about seeing consistent code across tightly-tied projects. – George Kagan Sep 19 '16 at 12:54
  • 3
    @timenomad What I mean is start off by introducing the easiest, least contentious guidelines first. Things like "use 4 space indents; do not use tab characters for indentation." or "save PHP files using UTF-8 format without BOM using UNIX line breaks" – Brandin Sep 19 '16 at 13:02
  • 1
    "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. My nature has always been to opt towards option a. – jleach Sep 19 '16 at 13:31
  • 8
    Have you [researched the benefits of coding standards](https://www.google.com/#q=benefits+of+coding+standards) and not only presented your opinions, but the information from other sources (especially those that you would consider reputable)? Have you talked to the rest of the team to get their thoughts - do they have a problem with the lack of coding standards? – Thomas Owens Sep 19 '16 at 13:35
  • 3
    I have been a PHP programmer for over a decade and here's my advice to you: **Run away and find a new job.** Otherwise you are going to suffer a lot. – Andy Sep 19 '16 at 14:36
  • 3
    I'm voting to close this question as off-topic because it is about people issues, not software design. – Robert Harvey Sep 19 '16 at 15:28
  • Do the best you can until the team grows. As lead developer (how does that differ from "project lead"), you'll have some influence over new staff. They'll see the differences. It'll be possible to quantify differences as more work on the two code bases. – user2338816 Sep 19 '16 at 18:48
  • it **is** an opinion-based question, but we have questions here all the time about Agile and Scrum and other code management issues. i think this question is relevant for this SE group. @DavidPacker says "run" and while i might identify with that (working with someone else's smelly code is like sharing an apartment with someone who often forgets to flush the toilet), you might not have the option to "run away". then you have a issue of office/company politics and of power. – robert bristow-johnson Sep 20 '16 at 00:30
  • 1
    "*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 he refused*" -- Part of good leadership style is to (1) give decision making authority to other people; and (2) not to dive into micro management. If now you are responsible for the entire code base, then you should not need to ask in the first place. And higher management should not be bothered with it either. **Code quality, maintainability and secure coding are good enough arguments. Go and use them.** – JensG Sep 20 '16 at 08:52
  • Coding standards had been an issue for many years throughout my career. Nobody ever agrees because most rules are personal preferences . Anyways, coding standards became a non-issue once tools became available that would not only inform the developer of coding violations but MORE IMPORTANTLY had a way to automatically correct the coding violation. The means to automatically correct was critical because it removed the burden of adhering to arbitrary standards that the developer may not agree with. Tools that simply reported the violation were not effective. Does PHP have such a tool? – Dunk Sep 20 '16 at 15:40
  • 1
    He sounds a bundle of fun. You could play passive-aggressive and introduce subtle bugs that would violate PSR2, but not his rules, then play the "the static analyser's warning was missed because of all the other inconsistencies it was flagging" card... – Synchro Sep 27 '16 at 19:11

10 Answers10

14

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.

jleach
  • 2,632
  • 9
  • 27
12

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.

dagnelies
  • 5,415
  • 3
  • 20
  • 26
  • 5
    Having worked professionally a very large PHP codebase, I can tell for a fact that the PSR coding standards don't simply serve to have readable code, but are also the *only* way to create anything resembling "safe" PHP code. – Rhymoid Sep 19 '16 at 16:51
  • 2
    @Rhymoid Agree completely. He insists PHP's forgiving nature is to be embraced and used to its fullest! But all it does is create bugs. – George Kagan Sep 19 '16 at 16:59
  • 2
    (Ack. As it turns out, you need a heck of a lot more than just the PSR coding standards to stay away from PHP's pitfalls.) – Rhymoid Sep 19 '16 at 16:59
  • 1
    @dagnelies I can see why he wouldn't care as much about the code, I just can't accept it - as the task of maintaining it falls on me. – George Kagan Sep 19 '16 at 17:03
12

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).

Lightness Races in Orbit
  • 8,755
  • 3
  • 41
  • 45
Jared Smith
  • 1,620
  • 12
  • 18
  • @timenomad fair enough, my own situation is a little different too (my boss's boss while not a programmer is a linux guru). But lots of people will see your question and apply at their Fortune 500 with disastrous results for us all. Either way, good luck I hope you get things tightened up or find a way to a shop with more mature practices. – Jared Smith Sep 19 '16 at 17:18
  • I need to add a disclaimer :) Thanks, same to you! – George Kagan Sep 19 '16 at 17:20
  • In the end it all boils down to workplace politics. I agree with this answer completely, but if you feel that you're capable of handling the politics surrounding the situation, you can actually make that work out well to your personal advantage as well as for the company/project. If.... big if. – jleach Sep 19 '16 at 18:43
5

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....

Newtopian
  • 7,201
  • 3
  • 35
  • 52
Walfrat
  • 3,456
  • 13
  • 26
  • 1
    That doesn't do much good in trying to proactively set up for a successful project. In fact, it's not much better than saying "ok, I'm lazy too, so screw it, let the project go to hell." – jleach Sep 19 '16 at 13:41
  • 1
    That's a fair point. I read it as you were suggesting to point out the fault was caused by his coding pattern. – Matthew Whited Sep 19 '16 at 14:26
  • 1
    @MatthewWhited I have edited to make sure that no one else would understand that. – Walfrat Sep 19 '16 at 14:30
3

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.

gnasher729
  • 42,090
  • 4
  • 59
  • 119
  • 1
    ...wow, I'm surprised someone actually downvoted this. I'd vote it ten times up. – dagnelies Sep 19 '16 at 15:49
  • About priorities - he also likes to do micro-optimizations in an age of super-fast computers. The app isn't a missile launcher too.. – George Kagan Sep 19 '16 at 15:52
  • 1
    @timenomad "We can waste cycles" != "We should waste cycles". I think the bigger issue with micro-optimization is that it is a complete lost cause in most scripting languages. At best it tends to do nothing due to the abstraction, at worst it can add overhead. –  Sep 19 '16 at 16:43
  • 1
    @timenomad if it'll take you just one day there should be no discussion, as you are now the lead developer for both projects and since this is not a large strategic decission it shall simply be your choice. – jjmontes Sep 19 '16 at 16:45
  • 1
    Code primarily exists for your coworkers (and you, in a month/a week/after your hangover), not for the compiler/interpreter. Adhering to coding standards is how you explain processes clearly to your co-workers, and consequently, it is relevant to getting along with your co-workers. – Rhymoid Sep 19 '16 at 16:54
  • 1
    Upvoting because I've seen coding standards wars do huge damage to interpersonal relationships and team morale. – david25272 Sep 19 '16 at 21:55
2

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.

Joop Eggen
  • 2,011
  • 12
  • 10
2

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:

  1. Has a business motivation in addition to the technical perfectionism motivation
  2. Will not be viewed as a large time investment (because it's not!)
  3. Will establish good stylistic habits precisely because you and your team are doing it over time
  4. 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?).

durron597
  • 7,590
  • 9
  • 37
  • 67
  • I don't know about PHP specifically, but in a lot of cases automated tools can handle these kinds of things in minutes and then everyone can use the new style going forward. – Casey Sep 19 '16 at 20:03
1

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.

Steve Jessop
  • 5,051
  • 20
  • 23
0

[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.

Solomon Slow
  • 1,213
  • 9
  • 14
0

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.

Schwern
  • 800
  • 5
  • 8
  • 1
    @timenomad I've heard many variations of "the automated styler won't get it exactly right" and I've been the one to say it myself. Some ways out: adapt the styler via config or even patching it; argue that it's a lot of effort to ensure the special style is applied consistently by humans for a small gain; argue that the big wins of style automation outweigh the small losses; argue that automated stylers can do even more than humans and editors can. To that last I'm thinking beyond syntax style and into full static analysis for common mistakes like Perl:: Critic. – Schwern Sep 19 '16 at 21:28
  • 1
    @timenomad Finally, your jobs are to write business logic for the company. Anything else you do is a waste of precious company time and money. Every extraneous thing you can offload on a free third party tool saves the company money. If a beautiful and unique snowflake style, even if it's arguably 1% better, means you have to spend precious programmer time and arguments on it, then you are wasting the company's money and not doing your job. – Schwern Sep 19 '16 at 21:34