No Compromises

Where are some good starting points for modernizing and improving a legacy codebase? Joel and Aaron discuss a few ideas where you could start.

Sign up for our Laravel tips newsletter!
Does anyone read these? Tweet @jclermont if you see this.

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. In one of our earlier podcasts, we had talked about taking over a new code base. Maybe some legacy code and what's the most important thing to kind of start testing. I think the idea there was to kind of get an insurance, or a nice comfortable blanket around it. Making sure that you know that these changes are... or at least this app is going to work fine when you make a of couple changes. Now that we've written some tests around an application like that, we have an interesting choice. We can continue down that road and kind of get maybe less and less return, and maybe start refactoring a few things. Or there might be other things for big wins, more important wins that we could do. I thought I'd ask you, Joel, if you've written some tests around and you're kind of comfortable with the tests suite you have, again it's not perfect, what's some of the next steps that you might take with a legacy code base?

Joel Clermont (01:07):
One of the places I universally like to start is to find and weed out dead code. There's a controller hanging around that there's no routes going to it. Or there's these models that exist but there's not even any tables or migrations or things for them. That might sound kind of weird but I've yet to inherit an app that didn't have at least several things in that category. The reason I find that helpful is because, then you can kind of take a step back and look at what remains and decide, well, what needs the most attention? But if you're looking at everything and then you start working on something and you realize, "Oh, this isn't actually used anywhere, I can just delete it," I'd rather know that upfront, get that stuff out of the way. Like you mentioned, it's a quick, easy win.

Aaron Saray (01:56):
I want to play a little bit of devil's advocate on that. I guess you're making it sound, to me, like when you get this code base you actually understand what it does. I guess that's my concern. Is when you say things like, "Well, I know this is not in use." There's a myriad of different ways that code can be in use, so maybe then... I guess a better question is, how do you even determine that code isn't in use?

Joel Clermont (02:22):
For me, the way I think of Laravel application and the way I parse it and review it is essentially from the routes file in. If there, if I look at the routes file and there is nothing going to a given controller, well clearly that's not in use, right? There's no secret way of calling a controller without a route being there. I mean, I guess you could put public methods on it and then new it up in some other class, but thankfully I am yet to be exposed to anything quite that bad.

Aaron Saray (02:59):
Actually...

Joel Clermont (03:00):
All right, let's hear it. Let's hear the horror stories.

Aaron Saray (03:04):
Actually, isn't there a way to call controllers and actions with some sort of action helper that allows you to say, "Run this action."?

Joel Clermont (03:12):
Sure.

Aaron Saray (03:12):
And that won't be in the routes then.

Joel Clermont (03:16):
Yeah. Are you talking about a redirect or something returned from somewhere else? Or, maybe building a link from a view, a blade view?

Aaron Saray (03:27):
Yeah.

Joel Clermont (03:29):
Okay. I mean, yes, I don't just look at the routes file in isolation then be like, "I'm going to start deleting stuff." But to me, it's pretty rare to have something not defining your routes at all but yet be in use in the app. But to your point, yes, you would not want to just go off and start deleting stuff.

Aaron Saray (03:51):
Well, I'll tell you one of the things that I do in a situation like that. Is instead of deleting those files, what I will do is I'll put some login into those files. For example, inside of a controller where there's a method that I'm pretty certain isn't in use, the first thing I'll do is I'll log to the very top of that. Something like, "I was called and I am XYZ method." And at least then let the application run for a little while just to verify that all these things aren't there. Then maybe if you use a little-

Joel Clermont (04:22):
In production you mean?

Aaron Saray (04:22):
Yeah, in production. If you use some sort of little keyword, like to do, remove or something like that as part of your log, then you might be able to track all those down in a month or so in the code base and say, "All the places where I've done this, I don't see any logs for these, so none of these are actually used."

Joel Clermont (04:42):
See, I thought you were going to say, you were going to comment out the controller and just leave it in Git for the rest of time because-

Aaron Saray (04:49):
Oh, no.

Joel Clermont (04:51):
... I know how much you love that. You know, when you see copies of files and it's like, "Trust your version control." All right, yeah I like that approach. I've never actually tried that but I could see how that would be useful. All right. For the sake of argument, let's say that we've weeded out the dead code by whatever means we feel is useful. The next thing I like to do... Again, I sort of have this route controller mentality that's kind of like the main driver of the application, is I will look at cleaning up those routes. Specifically, I like to default to the convention of the resourceful controller. You have your predefined seven methods and I will try to start fitting those into that shape. So, if there's a controller that has 20 methods, that's probably going to get my attention and I'll look at that and understand that.
Generally, the place I start is I'll identify things that really should be split off into a single action controller. If there's something that's handling some process type thing outside of the normal CRUD application, I'll break that off and name it and move it and then test it too.

Aaron Saray (06:08):
Maybe I'm just feeling like being that guy today. I've already said, "Well, actually..."

Joel Clermont (06:13):
Well, actually-

Aaron Saray (06:13):
I've argued with everything and I've pushed up my glasses in a very nerdy way.

Joel Clermont (06:17):
Yes, you have.

Aaron Saray (06:19):
My question along those lines is, what does that get you for the amount of effort you've spent on it?

Joel Clermont (06:27):
First of all, I would say generally speaking, it's a pretty mechanical change, right? There is some risk involved. Again, it depends on how bad this app is. If it's not using route helpers or named routes or things like that where you have to do a lot of string parsing to find where this thing might be called from, that's a little more work. But assuming it's not a total train wreck, it's just kind of got out of hand and grew beyond what I consider to be reasonable. That would be my first point, is it's not a major change. You're actually not writing new logic, you're just moving things around. But what I get out of that is, number one, I start to get real test coverage. We talked about starting off with smoke tests and happy and critical paths and things like that. But once I move it, I'm sort of taking more ownership and I'll write more complete test coverage for the thing I move.
Second, I just made it less daunting when I want to add a feature or work in this code base, it's sort of fitting more into the shape of an app I would have built from the start. I find that a nice return on investment for me as a developer. Of course, the business, it's hard to go to them to be like, "You know what? I spent three weeks and I moved all these controllers around and it looks beautiful." So you're kind of pairing things together. It's a little bit of cleanup here, making it a little bit nicer than you found it but in the meantime, actually doing real work, fixing bugs, writing features along with it.

Aaron Saray (07:53):
I think I agree with you. And what I would say, one additional bonus you get there is when you're probably moving those around, you're just getting some exposure to all of the code and all of the stuff too. Not only are you... I mean, don't move things around just to learn about the app, that's not the point. But if you are going to be refactoring to more of a convention, you get to touch all these different methods and at least kind of scan over them. I can't tell you how many times I've done something completely unrelated in the code and then remembered a couple weeks later, like, "Oh, I thought there was something somewhere in one file somewhere that said XYZ." And be like, "I better check." And sure enough it was something that you barely touched, but since you had scanned it with your eyes somewhere in the back of your mind it stuck.

Joel Clermont (08:41):
Yeah, for sure that is definitely a side benefit of doing that.

Aaron Saray (08:46):
You said basically then, moving controllers more into conventions, getting rid of extra routes. What other things?

Joel Clermont (08:55):
You want me to tell you all my secrets?

Aaron Saray (08:57):
Yeah.

Joel Clermont (08:59):
Okay.

Aaron Saray (09:00):
I like to work in threes, so what's the third?

Joel Clermont (09:03):
All right, I can give you one more. I'll give you one more. Another one I like to look at are the models. This is obviously a matter that's somewhat subjective, but I find in these legacy apps a lot of logic tends to end up in the model that isn't really model specific logic. It might belong better somewhere else, it might be something that was written in a much earlier version of Laravel and now there's something in the framework that does what that function is doing. So that'll kind of be my next pass. I've looked at the routes to controllers, now I'll kind of dig down into the models and go through them and see, "Where are these used?"
This is kind of a silly thing, but sometimes something will have a name that makes no sense. Because it started out as one thing and then it grew as the app grew and now... But the name never changed, nobody ever took the time to refactor it and rename it for its new purpose. So cleaning up logic in the models, renaming them where it's just like a total disconnect from what the thing actually is currently. That would be my next of the big three to get into.

Aaron Saray (10:13):
Cool. Well, I think you kind of touched on this too, but I want to double back on it. Is that you're not just moving stuff around to move things around, and you're trying to gain something from this and make it easier to follow. One of the things that I like to bring up those is... Obviously I have extensive experience working with teams. When you're in a team though, you have to have these conversations with your team before you just start doing it.

Joel Clermont (10:39):
Oh, yeah.

Joel Clermont (10:39):
Because otherwise, you're going to just start moving stuff around, everyone's going to be moving things around with their own opinion. So this is a good example of, "These are great tips, but it depends on how you're working on your project too." The tips get implemented differently based off if you're working by yourself or with a team. If you're looking at doing something like this, you might have a conversation with your team or with your dev manager or something like that and say, "This is my intention and this is the steps I'd like to take. Does this seem like something that makes sense for our group?"

Joel Clermont (11:09):
Yeah, that's a good point because... especially if you're joining the team, right? A lot of times that's how we get introduced to what I would call a legacy code base. Is, we're joining the team. The people on that team, they've internalized those pains for so long that they've sort of stopped feeling them. But a lot of times I'll bring something up like, "Why is this have this name?" Then there's a story, "Oh, yeah, we should have done that three years ago," but that's useful to get some outside perspective. You come in fresh and you're asking these questions and the people on the team understand why. Now, sometimes, to your point, they'll explain why that might seem wrong but that actually is the right thing. Or, they'll explain why that hasn't been done, because it's too risky or whatever. But, yeah, team communication definitely an important aspect of this for sure.

Aaron Saray (12:03):
I have an ethical question for you, Joel. Do you think it is ethical for all these fitness people to be hawking fitness machines that they don't particularly use? Or, for any of us really to recommend things that we don't use?

Joel Clermont (12:20):
Oh, boy. I don't think there's an ethical dilemma in recommending something you don't use. Because, for example, a doctor may suggest you take a medication that they don't take. You know, what I mean? Somebody can see the benefits of something for you and not necessarily use it themselves. However, I would say there are definitely circumstances where people are hawking things purely for a commission or money and it is very disingenuous and it probably would cross that ethical line.

Aaron Saray (12:50):
I think you can tell too. If you haven't met me, I am probably the size of three humans and if I were-

Joel Clermont (13:00):
Stacked on top of each other or side by side?

Aaron Saray (13:02):
Not tall, no. If I was hawking the fitness machine, like the Chuck Norris fitness machine or something, you'd be like, "Uhm," maybe that's ethical, but I certainly wouldn't really help it sell. "Trust me, where's my cheeseburger?"

Joel Clermont (13:20):
Does Chuck Norris have a exercise machine?

Aaron Saray (13:23):
Well, wasn't there someone who had one?

Joel Clermont (13:26):
I'm sure there is a person that has a machine, yes.

Aaron Saray (13:29):
Was that that guy, Bowflex, he has one.

Joel Clermont (13:34):
Okay. No, I think what you're saying is less an issue of ethics and more an issue of credibility.

Aaron Saray (13:44):
Oh, yeah.

Joel Clermont (13:44):
Like if there's this super buffed dude selling me a machine that's kind of for beginners, that's different than somebody that clearly does not enjoy exercise being like, "Oh, you should really get this machine." I will not be selling exercise equipment either by this criteria, just for the record.

Aaron Saray (14:03):
Are you a sucker for those things though that are demonstrated really well? Like, when you go to a festival or something and they show you those pans that somehow can cook everything?

Joel Clermont (14:15):
Yes.

Aaron Saray (14:15):
I'm like, "I certainly use one pan and I throw it in a dishwasher and destroy it in a month. But those look awesome and I'll bet I'll take care of those." No.

Joel Clermont (14:24):
It's like a real-life infomercial, yeah. Especially if the person knows what they're doing, they could be quite entertaining and hard to resist at the end.

Aaron Saray (14:34):
Did you like Joel's three steps and want to learn the six additional secret steps? Or even more that he won't share?

Joel Clermont (14:40):
Then head over to our website, at nocompromises.io/tips and sign up for our weekly newsletter.