Spotting Naming Smells0:00
I'd like to talk about method naming a bit more, because as it turns out, the names you choose will often be your first indication that something is begging to be extracted or changed. So take a look at this class here, slightly contrived, but it'll work for the lesson. We have a UserProgress class that accepts a User. And then if we take a look at the methods, at least to start, most of them relate to a watch later list. So addVideoToWatchLater. Or removeVideoFromWatchLater. And by the way, watch later would be a list of videos that you want to watch in the future.
Or remove it. And by the way, watch later would be a list of videos that you want to watch in the future. Anyways, here's another one. Is the video currently on the User's watch later list? Or toggle it? Or get all of them? Or flush them? Or get the cache key? So again, this is confusing to me, because the class is called UserProgress, but all of the methods are related to a watch later list.
So again, this is confusing to me, because the class is called UserProgress, but all of the methods are related to a watch later list. And this is a trap you'll often fall into. If you don't choose the right name for your class, that decision will cascade down to all of the names of your methods. And this is how we end up with methods that, in many cases, are three words or more. Add watchLater. Remove watchLater. IsWatchingLater. Even if you don't know exactly what the problem is, it should feel a little off, right?
Renaming the Class1:15
Is watching later. Even if you don't know exactly what the problem is, it should feel a little off, right? So let's see what we can do here. Now as it turns out, I do have a set of tests for UserProgress, and we can see they're all passing currently. So I do have free reign to make some tweaks here. So it sounds like the class name is not correct. Alright, I'm going to change it. So I will rename this class to what it really seems to be. It's a class for managing your watch laters.
So I will rename this class to what it really seems to be. It's a class for managing your WatchLaters. So why don't we call it WatchLaterCollection? Or WatchLater, you know, whatever you want there. I'm going to stick with WatchLaters. Now do note, because I'm using phpStorm, that change should automatically be reflected anywhere that I instantiated this class. So I do get green if I run that again. Okay, so now take a look.
Simplifying Method Names2:02
So I do get green if I run that again. Okay, so now take a look. Because we've chosen a more appropriate class name, notice that now our methods feel overly verbose. Think about it. If we have our User instance, and we want to fetch an instance of this WatchLaters class, you might have an accessor or a method, whatever you want. But then notice, if I wanted to call this method, we'd end up with something like this. User, WatchLaters. It instantly pops out at you.
User, WatchLater. It instantly pops out at you. Hmm, that didn't feel right. Way too verbose. So instead, well if I have my WatchLaters here, what am I doing when I call this method? I'm adding a new WatchLater. Why don't we just call it Add? And immediately, that feels and reads better. And notice, we'll make that change now. Simply by choosing a better class name, we've reduced this method name down from three or
And notice, we'll make that change now. Simply by choosing a better class name, we've reduced this method name down from three or four words to simply one. add. It reads better. Let's keep going. I'll scroll down. The exact same thing here. I no longer need to be clear about what we're removing, because I already have an instance of that.
I no longer need to be clear about what we're removing, because I already have an instance of that. So I will rename this to simply Remove. Okay, next, IsWatchingLater. Exact same thing here. But I can't just do Is. That wouldn't be right. So let's write it out instead. Once again, User, we're going to get an instance of this class. And right now, you have to do This, which we know we don't like.
Once again, User, we're going to get an instance of this class. And right now, you have to do This, which we know we don't like. So I don't want to do Is. It sounds like we just want to say, for our WatchLaters list, do we have one for this video? Pass. All right. Once again, we had a method name that was three or four words. Now it's one. Let's continue just a little bit more.
Now it's one. Let's continue just a little bit more. Toggle WatchLater. All right, let's just change that to Toggle. And then here, let's update the string versions here. All right, a little bit more. This one can simply become Get. And then finally here, WatchLaterKey. Class like that should simply be Key. And then finally, the last one here, this simply flushes or deletes all of the relevant
Class like that should simply be Key. And then finally, the last one here, this simply flushes or deletes all of the relevant Redis keys. I'm going to change that to flush. All right. So now, take a look at the method names. Doesn't this instantly feel better? Add. Remove. Pass.
Remove. Pass. Toggle. Get. Key. And Flush. It turns out this class was basically a collection, but it wasn't immediately clear because we had the wrong class name. User progress did not make it clear what we were working with. But as soon as we found a more appropriate name, everything else fell into place.
Updating Tests After Renames4:41
User progress did not make it clear what we were working with. But as soon as we found a more appropriate name, everything else fell into place. Okay, so if I run my tests again, we do have some issues here, but only because of the name changes. So we can fix that very quickly. Let's scroll down and update some of these. So yeah, you'll see right here where we clean up after ourselves. That should now be Flush. Okay. Next, a video can be watched later.
Okay. Next, a video can be watched later. So I'll go through this one very quickly with you, and then I'll fast forward. Given we have a Video, and the User adds that Video to their watch later list, well, if I then get an array of all of our watch laters, we should have exactly one item there. So if I give that a run, it does fail, and again, that's because we haven't updated the names here. add. get. And now that returns green.
Let's do one more together. It may not be added twice. All right. If I have a video, and the user adds the video to their watchLater list, if I then fetch an array or collection of all of the watchLaters, there should be exactly one item. And then, if we add that exact same video again, it should be smart enough to know we're not adding it two times. So if we try it one more time, the total number of items should still be one. And we get green. Okay.
Fixing Final Failing Test6:08
And we get green. Okay. So I'm going to fast forward and finish all of these on my own. Okay, so I've reached this point. It can be toggled, but this one is still not working even after I've updated the method names. Given I have a video, and I toggle the watchLater status, if I check if we have one for the video, it should be true. But if I then toggle it again, this time, it should be false. But if I run it, it fails.
But if I then toggle it again, this time, it should be false. But if I run it, it fails. Okay, this is our final step. We'll take a look at that method name here. And notice here, we're still doing a call to isWatchingLater. Looks like PhpStorm didn't change that for us. So we'll change that to has, and if I run it again, now everything is passing. And there you have it. Much, much better.
