Avoid painting yourself into a corner

We've all been there: a thorny piece of code works, but it seems like there must be a better way to simplify it or make it more readable. We share one example when this happened to us, and how code review led to a better solution.

Want another set of eyes on your code? We can help! Visit our site and schedule a free call with us today.

• 00:16 The problem: this code works, but I don't like it
• 01:17 What we mean by "I don't like this code"
• 02:22 A specific example of code I didn't like
• 03:52 Code review sparks a discussion, and a solution
• 05:20 Principle: don't get locked in to your first approach
• 06:03 Principle: the benefits of code review
• 07:22 Don't be defensive, and the code reviewer isn't always right
• 08:03 Silly bit

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):
This is Aaron.

Joel Clermont (00:16):
I've had this experience where the only way I can describe it is you've worked on a feature or a particular piece of code, and at the end of it, you kind of feel like you've sort of painted yourself into a corner, for lack of a better term, where the whole time you were working on it, it just felt harder or more convoluted or more complex than it should be, and it took longer than it should. I mean, sometimes that's just a general thing, but you just don't feel good about what you're doing, even though it's working. It actually is. When you're done, the tests pass, the feature works. Have you ever experienced that, Aaron, or am I on my own on this one?

Aaron Saray (00:58):
Oh yeah. Tons of times, when you output it, you're looking and you're like, "Yeah, that solves a problem, but what?"

Joel Clermont (01:04):
I don't like this. I want to give a tangible example I had in mind and then maybe I can say how we resolved it, and I resolved it with your help, Aaron.

Aaron Saray (01:17):
Before you go on, I want to make just a quick aside, the not liking it isn't about this code doesn't look beautiful or elegant or anything like that.

Joel Clermont (01:25):
Okay.

Aaron Saray (01:26):
It's not about there's too much...

Joel Clermont (01:28):
Too many lines.

Aaron Saray (01:30):
There's like three lines extra in this control, right?

Joel Clermont (01:32):
Yeah.

Aaron Saray (01:33):
It's actually hard to understand. I'm looking at it and it's not clear. I even know myself if I were to go away from this project, which I will, and three months later I come back, I'm going to look at this and go, "What was I thinking?"

Joel Clermont (01:44):
Right?

Aaron Saray (01:45):
I don't get it. This idea that some comments are good, but if you need a lot of comments, then maybe it's too complicated. We're talking about things like that where it's not perfect. Fine, you'll never get perfect style code, leave it alone. This idea of, "Oh, that seems a little hard to follow," or "Is that really working how I expect it to?"

Joel Clermont (02:07):
Yeah. Is that the best way I could have done this? Surely Laravel has a better way of doing this. That those are some of the thoughts I have. Yeah, you're right. It's not just this more subjective, "It doesn't look right to me." It could be more elegant. It's a little more tangible. This just isn't satisfying. Here's the example I had in mind. We were working on an API that surprise, it had users, but a certain type of user could make payments to other types of users. Just not to get too much in the weeds in the business, but that was the basic flow. We wanted to have a policy in place where a user for a given job could not pay this other user who did the job for them more than once. It was kind of a sanity check that we never double charged anything financial.

(02:57):
We work really hard to avoid even things, "Oh, that'll never happen." We really want to make sure it'll never happen. As you might expect, we were in the user policy, right? That's where this is applying. Can this user make a payment? User policy. Users have a relationship to payments. It's more than one, but it kind of goes through this job table and it was a little complex relationship. Then the more I tried to layer it on, because again, not to get too much into the weeds, but for a given job, there could be multiple people who did it. It wasn't just, "Oh, you can only do one payment per job, but it's only one payment per job, per worker of that job." I had this just gnarly user arrow whereHas payments with all sorts of subqueries, and I didn't like it, it worked. It was complex.

(03:52):
I think it was at the code review where you're like, "Come on, there's got to be a better way to do this." I'm like, "No, show me." I think you said, "Why are you doing it from the user side? If you do it from the payment side, this would be a lot simpler, wouldn't it?" I was mad at first, because I'm like, "Shut up." Then I'm like, "Yeah, that actually would make it a lot simpler." I'm coming at it from the user side, the user model starting there because I'm in the user policy, but there's no rule that I have to do that. Sure enough, when I flipped it and I started with the payment model and then did relationships back to the job and to the user from there, it was so much simpler. I mean, it still wasn't like a one-liner, but it was measurably better and I felt a lot better about it.

Aaron Saray (04:39):
Yeah, it makes sense. A lot of times, especially if you have the user, there's a lot of other relationships you're like, "Obviously, I'm going to come off this side of it."

Joel Clermont (04:47):
Yeah.

Aaron Saray (04:48):
You don't always have to, whether it's in the policy or permissions or whether it's even retrieving data. There's so many times I've seen people write a controller where they want to get the current user's stuff and then they make this really complicated sort of relationship and paging on relationships, which is kind of tough, where I'm just like, "Why don't you just do paging on the actual model with a quick where?

Joel Clermont (05:12):
Exactly. Why don't you just do that? Aaron's like, "Why don't you just do it the right way?"

Aaron Saray (05:17):
Just do it. Why would you ever?

Joel Clermont (05:20):
This was kind of one tangible example. Maybe it only applies to me in this one project, but I think the principle that lit up in my brain that day was, your first way of even approaching a problem isn't necessarily the right one. Sometimes, especially when you're feeling like, "Ah, this just doesn't feel right." That's a chance to step back and be like, "Is my fundamental starting point wrong? Could I avoid this problem by thinking differently than where my brain first went when I sat down to code this feature?" The other thing I thought was kind of a nice takeaway from this too was the benefits of code review.

(06:03):
In this case, maybe in a perfect world Aaron, and I would've bugged you before I finished coding it up and submitting it, but at the least you saw it because we review each other's code and to your credit, you called it out, you didn't even have an answer like, "We'll do it this way." You just like, "What about this? Or what about that? Have you tried this?" I just thought, some developers look at code reviews as this hurdle to cross or this thing that slows them down or gets in their way or this point of contention. I just think this is a really positive angle on the code review process.

Aaron Saray (06:41):
You have to watch how you feel about your code reviews and stuff. It's kind of a tangent, but I think one of the things that probably people wouldn't know is if you're reviewing my code and you find something, I'll sound and look very angry with lots of profanity and everything, but if you're just like, "Oh, you mad?" I'm like, "No, I'm happy." I'm happy you found this stupid thing and then just go on another bleep filled rant. I'm a cross between ashamed or just upset with myself and just, "Oh, of course. Why did I miss this?" It can be easy just to get mad or you can realize what are you actually frustrated with?

Joel Clermont (07:22):
Right. Yeah. There is this initial defensive posture I think we all take when somebody criticizes our work, even if it's constructive criticism, I think it's human nature. If you can see past it, and I know you and I too, we've had these things where there'll be a comment on a pull request and the other person will push back and be like, "I actually think this is better than what you're proposing." Or, "I tried that and I didn't like it for this reason." The code reviewer isn't automatically right, but it starts a conversation and being open to that conversation has some benefits.

Aaron Saray (07:58):
Yeah. It's a code reviewer, not a code corrector.

Joel Clermont (08:00):
That's right. I think being a software developer causes you to view the world in a different lens maybe than the average person. By the way, I'm not saying we're above average. We could be below average too. I'm just saying it's different. I'll give you an example of this. I've been using an app on my phone to keep track of what I eat throughout the day. It tracks calories and all this stuff. It has a huge database of foods, so you don't have to type in all the numbers. You just put in string cheese or whatever. Every once in a while, we have our own recipes we make and things that aren't in there, and so you can create your own foods and then when you're creating it, there's an option that says, "Do you want to share this?" There's a community portion of this food database.

(09:00):
You come across these other foods that people enter when you're searching for things. I'm like, this app needs a spell checker. I'll just give you some examples of things. I was mistyping something, because they should have a spell checker on the search field again, which they don't. I somehow typed in injure I-N-J-U-R-E, and it came back with injerale. Somebody that typed in a particular brand of Ginger Ale soda. It just left off the G and also spelled it with a J in the middle. There's that.

(09:45):
We grilled out the other day and I was starting to type "ribeye", but I typed it wrong, and so a rubeye steak came up R-U-B. I'm just like, "All right." The validation guy is going nuts in my head. Clearly nobody's reviewing these. There's no spell checker.

Aaron Saray (10:03):
Right.

Joel Clermont (10:04):
All of that. Just want just one more example, which was my favorite was peunats, P-E-U-N-A-T-S, instead of peanuts. I'm like, "You know what?" I actually entered that one because it made me giggle so much. I'm like, "I ate some peunats today.

Aaron Saray (10:26):
If you need another set of eyes on your code or maybe you work alone and need a code review, we can help.

Joel Clermont (10:32):
Head over to our website, nocompromises.io, and there's a book-a-call link where you can talk to us and see if we can help.