Pick a coding standard, any one will do

Where should I place this curly brace? Trailing commas or not? Have you encountered disagreements about coding standards on your team. In this episode we explain the benefits of coding standards, how to adopt them on your project, and discuss the two most popular tools in the PHP community for enforcing them.

Download our free eBook of Laravel tips: A Little Bit of Laravel

Creators & Guests

Host

Aaron Saray

Host

Joel Clermont

What is No Compromises?

Two seasoned salty programming veterans talk best practices based on years of working with Laravel SaaS teams.

Joel Clermont (00:00):
Welcome to No Compromises, a peek into the mind of two old web devs who have seen some things. This is Joel.

Aaron Saray (00:08):
And this is Aaron.

Joel Clermont (00:16):
In previous discussions, we've talked a little bit about how to inherit a new code base. Whether it's you taking on a project from a previous developer or joining a team and getting your wits about you in that new code base. I don't think we talked about this before but I think it'd be worthy of discussion. The idea of having a coding standard and some consistency to how code is formatted and organized, and things like that. Aaron, you always have thoughts on things. What are your thoughts on coding standards and how to approach that? Especially in a legacy code base.

Aaron Saray (01:01):
Well, I don't remember starting out learning the program thinking, "Boy, I want to come up with some standards and doing things," I think I just did things. And it's funny that when you start getting involved with other people or teams or whatever, you start to realize that maybe the way you did it isn't the way that everyone does it.

Joel Clermont (01:19):
Sure.

Aaron Saray (01:19):
And I think that's why you end up having standards that are defined and described and here is a way that you could do it. I think the important reason to have a coding standard that you adhere to is so that you don't waste time on things that are less important, like trying to convince someone your way of writing this is better, or seeing them change stuff and things like [inaudible 00:01:47]. I can say very much specifically that before I understood the value of code standards, I would try to convince people that my way was better. And there's a lot of time lost with something like that.

Joel Clermont (02:00):
Yeah. I kind of want to piggyback on that because maybe the argument against a particular coding standard is like, "Well, I don't like that. I like my braces here," or, "I like commas here." But personally, I've been on projects where the coding standard wasn't my default, like the way I would have written it naturally. I still prefer not having to think about it and not having to argue about it. Like, you're talking about it on a team. We're, "Oh man, this file was edited by this guy who does it this way," now I come in there and it's a constant battle and frustration. Whereas, once the standard is picked, even if you don't like every aspect of it, it's sort of a decided thing. You don't have to give mental weight to that every day as you're working in the code base.

Aaron Saray (02:47):
Well, it allows you to apply your art and your logic to something that matters, which is solving the business's problems not necessarily designing a different way of looking at the code. Now, when I say something that matters, I'm not saying that code style and beauty and things don't matter, but they make no money compared to if you solve a problem for a business, right?

Joel Clermont (03:13):
Yeah.

Aaron Saray (03:14):
So having a standard and having beautifully written code and things like that, making it easier to read, can make it more efficient. But there's a trade off at some point, how much more efficiency can you gain by making it look even better and even better? Versus saying that, this is just the way that we do it and then that's what we stick to. I also think it's important to understand that code standards help with something else I'm really happy about which is code review too. Part of a code review process is trying to find in all this code something that's either can be worked on, that can be learned from, that maybe is broken. When you have a code standard and everyone does it the same way, you can start to see patterns.
Then when the pattern is maybe out of rack or looks a little different, you have a place to focus on for your code review. Or, if all statements look this way, then you can actually see the statement content and not be stuck on, "Is this that kind of statement?" It's formatted this way. If there's always a brace on the next line for method declarations, but when you see it missing you know it's going to be the next line and that's what that tells you too. It's a method and you can look at it in that fashion.

Joel Clermont (04:32):
Yeah, I like the idea of patterns because after a while you do stop seeing the details of the syntax and you can focus more on the structure and the reason behind writing the code and what the important pieces of the code are. So, yeah, it might take a little getting used to if it's not your style but you'll get used to it and then you can focus on the more important things. With regard to code review, I'm envisioning you with a big chunk of code and you're counting how many spaces this is indented and you're paying a lot of attention to parentheses and things like that. I'm leading you here, Aaron, but how could you integrate this into a code review process in a way that's seamless and effortless?

Aaron Saray (05:22):
Well, good question, Joel. I wonder where you came up with that one from.

Joel Clermont (05:25):
Well, thank you.

Aaron Saray (05:26):
I think the leading here is that once you've kind of decided on a coding standard, there are tools that you can use to measure and/or apply that coding standard. Before we talk about the tools, I think there's two important steps to this. It is the measurement of whether you're within the standard and then possibly using a tool to alter your code so that it is within the standard. And those are two separate things. Different tools have their own strengths and weaknesses on them, but I think it's important to understand that the part of the code's standard process is those two are separate.
Also depending on the standard, there might be multiple ways to make the code within standard. So that's something else to keep in mind is, these automated tools, they might just pick one way. But there is sometimes wiggle room within even standards of how you might construct something, or if you don't like the way that the tool is applying the standard, you can change the way you've written your code or... You know, there's all these different things too. That's why I like to keep those separate is, there is a difference between seeing a foreign standard and then applying it.

Joel Clermont (06:42):
Yeah, the way I've seen those two jobs referred to is linting, that's sort of like the checking piece of it, and then fixing, which is the actual correction of the code to make it fit the standard that you've decided on. Let's stop dancing around this, what are the tools? Where do I go to get this set up? What's the best tool? There's lots of tools, what's the one you like? And is there one true tool?

Aaron Saray (07:08):
Well, there are two in the PHP world that I think of a lot of times. Just PHP Code Sniffer and PHP CS Fixer. They take a different approach to the process as well, that's why I separated out those two things. When you look at something like PHP CS Fixer, it kind of says it in its name. It's a code standard fixer. Its default way is to function in such a way that it sees the standard and then fixes it, that's all kind of built into the tool. If you want to measure whether you're within standard without altering the code, you have to add a parameter to the call to make sure it doesn't automatically fix it and just dry runs it. Whereas PHP Code Sniffer named very similarly-

Joel Clermont (08:00):
Not confusing at all by the way.

Aaron Saray (08:01):
Yeah, exactly. It will sniff the code or lint it, like you said, and then it comes bundle with another tool which will apply the fixes if you run it separately. Everyone can have their own ideas of which tool is better. I know that a lot of times Laravel community will use CS Fixer. I think there's a lot of good reasons to do that. Like, don't get stuck in the process. Write your code and then have the tool make it into something that fits within your system. I tend to prefer the PHP Code Sniffer, the one where the two stages are kind of separate and I think it has something to do with maybe a slightly mistrust with some of the tools. But also I feel like I want to be really responsible and proactive when I make changes to my code.
So when I've written my code I know what it is. When a tool modifies it, I don't necessarily know what that tool has done to my code, but I'm still responsible for it, so I like to keep those two steps separately. Then I can also say, one little side note here, in the past I've seen PHP Code Sniffer been sooner to the party to support things, like the newer versions of PHP and things like that. But I think they're catching up with each other and they're staying pretty current again.

Joel Clermont (09:19):
Yeah, I knew we were going to be talking about this so I actually looked at the project pages for both of them, and they're both quite active. And you're right, there's a little bit of a, I don't know if arms race is the right word, but one will get PHP 8 added, named attributes or something and one might get it before the other one. We've also found some benefit in actually using both. Because another thing that I've seen is different between them is they don't offer the exact same types of checks and rules. You might get like 95% of what you want in one of the tools, but the other tool has just those few extra rules that you want to make your code the exact way that you want it for your team. So you can use them both together.
Other than the confusion of, like... I was just looking at a source code here. There's .php_cs and then there's phpcs.xml, and then the tool names overlap a lot. So it can be a little mind bending at first to get your head around what is doing what, but there is value in both. I don't think we have a clear favorite. I know you said you prefer the one for its default operation, but we found value in having both of them for sure.

Aaron Saray (10:30):
Well, I think I would actually disagree with you a little bit. I don't see a lot of value in using both of them because I know that both of them can be extended through other packages. So out of the box I would say you're right, that together one does 95% of what I want, the other one does only 5%. But there are a number of other packages that you can install that add additional coding standards so you can just import certain things into them that solve the problem for me.

Joel Clermont (11:06):
Which one had the [inaudible 00:11:08]? That's the Code Sniffer, right?

Aaron Saray (11:10):
That's Code Sniffer, yeah. That package was able to basically put in all the rest of the things I was missing. So I'm not definitely saying either one is better or worse or whatever. I mean they've both come a long way too. But I think I would disagree upon seeing running both in the same project. I also think that'd be hard on your IDE if you configured that, how does it know which one to pick and stuff like that? Especially with some IDE is auto configured, so they read your project, understand what's in there and then it's going to try to run both at the same time, which one goes first?

Joel Clermont (11:42):
Your fan spin up and all of a sudden...

Aaron Saray (11:44):
Yeah.

Joel Clermont (11:45):
Maybe the two competing tools is how artificial intelligence finally gains sentience and takes over the world. Well, I just thought of it because some of our projects actually have both configured. But in terms of the tooling, the things that we actually run day in day out, you're right. We only use one of them.

Aaron Saray (12:05):
We we're talking a lot about the PHP tools here, like Laravel project probably has some JavaScripting it too. Not a thing I'm going to focus on too much, but there are tools there as well. Something called ESLint, which does that checking. And then you can extend that with something called Prettier, which again, kind of works hand in hand. That's a whole other conversation. But those are two tools that might work together to add dysfunctionality to the JavaScript side of stuff as well.

Joel Clermont (12:33):
Yeah, good acknowledgement, good to keep that in mind. So adding this in from day one of a project is going to be a little bit more clean than coming into year seven of a project with hundreds or thousands of source code files. What sort of strategies might be recommended if you want to introduce coding standards to your project and you're not quite sure where to start?

Aaron Saray (12:58):
Well, I think you have two sort of ways to do this. One, pick your coding standard, apply it to all your files and then make one giant commit and now everything's within standard. That may not be great but everything is within standard. Then you can add to your CICD some tool to say if it's out of standard no longer continue. The other is to change sort of files as you go. So if you touch a file, make sure it's in standard. There's even so much as just only the method that you touch was within standard. I can say that I've tried each way and I've found both to be effective in their own sort of right depending on the coding standard.
I'll give you one of these horror stories. I implemented a coding standard and the one thing I added in there was strip types. I applied that to every single file I touched but there were sometimes when the stuff wasn't under proper unit test or whatever, and I did not notice that some of the light equaling, not exact equaling, was now changed. Or, things would just crash because they were actually then expecting a certain type. But I also think that was a little bit beyond a code standard, that was a change of PHP coding level, I guess. I don't know how you'd want to say it. But I used a code standard tool to implement. So I guess that's where I would draw the line is, if you're going to apply something to all the entire project, it can't change the way that your project functions. Like, adding in maybe return types or perimeter type hints and stuff like that, actually does change the way the code will compile and function. While that's a great thing for a code standard, you can't just change that willy-nilly all over the place.

Joel Clermont (15:01):
Yeah. Odds are if you have a legacy code base where you're just now introducing some of these things, probably doesn't have the best test suite either, at least in my experience. Yeah, maybe separate those out, be a little more conservative with introducing that. But the other thing I thought of too is maybe if you are going to do the all-in approach to kind of communicate that with your team members. Try to find a point when there's not a ton of pull requests hanging out there, they're going to now all of a sudden have a bunch of potentially merge conflicts and things you have to resolve. But if the tool can do it for you automatically and your test suite passes and you're not introducing those behavioral changes you talked about, like type hinting or strip types or things like that, it's usually pretty safe to do it across the project. But your mileage may vary so be aware of that.
The one thing I also want to add and this, again, is an opinion thing. But personally I like it when CI will reject something for failing a coding standard. But I really don't like it when it automatically fixes it and adds a commit upstream in my GitHub or GitLab or wherever you're using for source code. I get why people do it, it's convenient, but I really personally find that adds a lot of noise to the process and I'd rather as a developer just have it rejected a few times. You remember, "Oh, yeah, I forgot to run this tool." Then everybody just does it as part of their routine, it's sort of like tests failing. You get notified about that but it's not apples and oranges. But it'd be annoying if GitHub is like, "Oh, I can fix that for you. I can make this test pass, I'll just comment it out," or, you know, something like that. Those sort of automatic things, Personally, I find a little annoying. I'd rather do it on my machine with the tools set up in the project for everybody.

Aaron Saray (16:58):
Yeah, I think you're right. That adds some noise but also, as I always come back to, you are responsible for the code and your branch. I would say that saying a tool made some changes for you out of your control is not something I'm comfortable with. You could argue, "Well, it does it locally." Well, I can review in the commit before I push it and see all of its changes. I don't necessarily like some tool altering my code once it's out of my hands. And then, of course, you could always set up Git Hooks and stuff like that to run this stuff before commit, make sure it's within... I guess, I should be real clear in there. Set up a Git Hook before commit to run the standard checker, not to fix the issues. Because then again, it's going to commit something that you didn't look at.

Joel Clermont (17:47):
Yeah, exactly.

Aaron Saray (17:54):
You know what's funny is, sometimes you have to take medication after surgeries or stuff like that?

Joel Clermont (18:01):
Mm-hmm (affirmative).

Aaron Saray (18:01):
And they always say, "Don't take this and operate heavy machinery."

Joel Clermont (18:06):
Okay, no forklifts.

Aaron Saray (18:07):
Yeah. And I know that means probably don't drive a car. But it feels like, "Yeah, I shouldn't be driving a forklift or a bulldozer," these are all things I can't do anyway. But now the pill tells me, "Don't do this." I just think, "When are we going to say things how they are?" Like, "Hey, this messes you up so don't do anything that requires motor skills." But I guess then if they said that, I'd be like, "Well, I'm not fixing engines so I don't need any motor skills."

Joel Clermont (18:37):
Oh, boy, you're going to be that guy. They should probably also include in there, not just heavy machinery but also don't sign any legal contracts or make any life changing decisions while you're on this medication.

Aaron Saray (18:51):
Yeah, don't text your ex. I mean, just don't do that. Stay away from your Amazon wish list and converting that into purchases. There's a bunch of stuff I suppose you shouldn't do on these medications.
In previous podcasts we've talked about our tips newsletter and a tip a week is fine. But wouldn't you rather just get them all at once?

Joel Clermont (19:16):
Well, boy do we have something for you? We've collected our tips into a free eBook which you can download. It's available at nocompromises.io/tips.