Identify Collection Refactor0:00
Let's begin refactoring the query class. Now one of the first things that jumps out at me is all these method calls, and they all follow the same shape. getSomethingCollection. Sort of like this. getXCollection. Notice getTrendingCollection, getRecentlyUpdatedCollection, getWatchlistCollection, and getInProgressCollection. Now if we take a look at any one of these, how about getRecentlyUpdatedCollection? Okay, all of those will return a plain old php object.
Now if we take a look at any one of these, how about RecentlyUpdated? Okay, all of those will return a plain old php object. But now notice how these have a title, a description, and series, very similar to our FeaturedCollections table. A title, a description. So the only difference is, for this series relationship, we're not storing those within a pivot table, those are being constructed dynamically, as you see here. So what if we returned a FeaturedCollection instead? That's what it is, so why can't I do that? Let's just see if that works.
Clean Up Auth Collections2:22
and there you go. So we are caching this query for a day. But instead of doing that, we could say, now, addDay. And Laravel supports that, and it just adds a little more clarity that we are caching this query for exactly one day. Everything's still going to work in this case. Next, let's scroll down here and clean some of this up. So it looks like we have some collections that will be available to everyone, but then other collections, as we talked about in the last episode, are dependent upon if the User is signed in.
other collections, as we talked about in the last episode, are dependent upon if the User is signed in. So there's no point in showing a watch list if you're not signed in. And the same would be true for in progress. So we do have that wrapped around in AuthCheck, but before we do a larger refactor, why don't we clean this up a little bit? I'd rather just say featured, prepend, and then this, getWatchListCollection, and then remove all of this. But now this is a little different. Remember, we only prepended that collection if there were one or more series associated.
But now this is a little different. Remember, we only prepended that collection if there were one or more series associated with the watch list, and we're not doing that here. So instead of doing this check here, what if we, before we return, just filter it down to only the featured collection where there's at least one series associated with it? And then what I'm going to do is call values, and that will rekey it. All right. This may be temporary, but at least it allows me to remove this. Now let's see if I can do the exact same thing here. I'll call prepend, this, getInProgressCollection.
Now let's see if I can do the exact same thing here. I'll call prepend, this, getInProgressCollection. And with any luck, if I run the tests, that passes as well. Okay, good. So now it's a little bit cleaner. Now another option we can do is store the AuthCheck directly within these calls, and we let them be responsible for it. So for example, if auth guessed, then just return. There's nothing for us to do. So we would do that here, and then in progress.
There's nothing for us to do. So we would do that here, and then in progress. So now if I scroll back up, I can get rid of that, rerun the tests, but yeah, it does fail. And that would make sense. In some cases, we are returning null from this method. So we run filter, where we try to grab a series property off of null, and it fails, as you see there. So again, we're taking this step by step. Let's first run a filter to remove any falsy values, and then do another filter, depending
So again, we're taking this step by step. Let's first run a filter to remove any falsy values, and then do another filter, depending upon if we have series. So if we run that, it passes. Okay, so now the main goal here is I wanted to get those conditionals out of the get method. Now notice at this point, we had a featured variable, and then we did some more with it, and then we did some more with it. I don't think we need to do that anymore. So let's get rid of all of that, and then same thing here, and then ultimately return and reformat.
So let's get rid of all of that, and then same thing here, and then ultimately return and reformat. Whoops. There we go. Run the tests, and that works as well. All right, still a lot going on here, but it's getting better. Next, at this point, you'll see we run filter and values two different times, as you see there and there. I don't see any reason to do it twice. So if I get rid of that, can I rerun it?
Extract Collection Classes5:35
I don't see any reason to do it twice. So if I get rid of that, can I rerun it? Yes, that works as well. Okay, next, we talked about this method getBlankCollection, and it looks like we're doing that one, two, three, four different times. So yeah, often when you encounter this, it's a signal that there are classes begging to be extracted, and for the general shape, the name of the class should be whatever x is. So in this case, getTrendingCollection, x is trending. So you'd create a class called Trending, another class called RecentlyUpdated, another class called WatchList, and then one last class called InProgress.
So you'd create a class called Trending, another class called RecentlyUpdated, another class called WatchList, and then one last class called InProgress. All right, let's give this a shot. I'm going to create a new class here. We'll start with Trending, and now if we switch back, I'm just going to go to this method and copy everything. So still lots of issues here I want to refactor, but one step at a time. I'm going to grab all of that, copy it, and then bring it over here. Now I don't know where this is going to go yet, so let's just throw it into the constructor and then move it where it belongs.
Now I don't know where this is going to go yet, so let's just throw it into the constructor and then move it where it belongs. Okay, so if we start, yeah, all of this looks like a way to fetch the series. So why don't we move that into a method called series, and then return that. Now I can update this, and now all that remains is returning a FeaturedCollection. But this is a little odd. We could do it like this, but this Trending class, it is a FeaturedCollection. That's what we decided earlier. So if we have an is a relationship, then why don't we try inheritance. Let's extend FeaturedCollection.
So if we have an is relationship, then why don't we try inheritance. Let's extend FeaturedCollection. Now I think there would potentially be issues with this only due to the fact that this would then be an Eloquent model. Now it works in our case if we just decide, well, we're not going to treat it like an Eloquent model. We're not going to run queries off of Trending. We're only extending it to hard-code some of these values. I think that would be fine, but also something to be aware of, and if that was a problem, we wouldn't take this approach.
I think that would be fine, but also something to be aware of, and if that was a problem, we wouldn't take this approach. We can at least try it. Now we don't need to return a featured collection because trending already is. So why don't we just hard-code what the title was, trending series, what the description was, and let me grab that. Bring it on back, and that allows me to get rid of this and this. The series should be accessible as a property, so why don't we make that getSeriesAttribute, and then finally cardTheme, and I wonder if that would work with Laravel. I'm not sure, but we'll do it like this.
and then finally card theme, and I wonder if that would work with Laravel. I'm not sure, but we'll do it like this. Okay, so now we'd end up with something like that. Anyways, if I run the test, it's still going to pass because at no point have we used this class, so that's going to be the next step. What if at the top, my first refactor is to new up our trending instance? Now I'm not doing anything with it, but if I run the code, it still passes, which means at the very least, nothing broke within that trending class. Let's now return it, run it, oh, and it fails, trying to call count on a Boolean. Oh yeah, that's because we have this section where we return false.
Let's now return it, run it, oh, and it fails, trying to call count on a Boolean. Oh yeah, that's because we have this section where we return false. So this is another example of why you want to be careful about returning different types from a method. In this case, it requires too much awareness. Sometimes it might return false, sometimes it might return a collection, sometimes it might return something else, you know, for some projects. It's too many things to be aware of and to accommodate. So why don't we just remove this entirely? If I were to comment this out, we fetch a set of series IDs, so that will be an array,
So why don't we just remove this entirely? If I were to comment this out, we fetch a set of series IDs, so that will be an array, and then it looks like those are collected, mapped over. We fetch each series on its own. I'm not sure why that's the case. Why can't we do findMany? I think it might be to retain the sequence and the order, but then here it filters out the falsy values anyways. So I think this is probably superfluous. Let's run it.
Refactor Remaining Collections9:57
So I think this is probably superfluous. Let's run it. Yeah, and that does work. So now we end up with something like that. All right, so if I come back, think about it. I can now remove all of this code, rerun it, and we have a good refactor. Great. Now let's do recentlyUpdated. And this time, I'm going to do it a little quicker. And in fact, why don't we just duplicate trending?
So we have our title is recentlyUpdated. And then the description. Finally, cardTheme. I don't think there is one, so I can remove that. Good. And then finally, update this. All right. So let's give it a shot. return new recentlyUpdated. Run the code.
Return new recently updated. Run the code. And that works as well. So I can remove all of this. All right, let's keep going. Let's duplicate it again for inProgress. All right. First up, excuse me, inProgress, all of this. So first, if auth guest, we're going to talk about that in just a minute. But we can take all of that, and we're going to move it down here.
And we still have this section here. We'll talk about that in just a minute. However, we could reverse this. If you're signed in, then return in progress. And I think that'll still work. Okay. Let's make a note of this, and then we'll come back. All right. Our last refactor, because I know it's getting repetitive. This will be watchlist.
And I think that'll still work. Okay. Let's make a note of this, and then we'll come back. All right. Our last refactor, because I know it's getting repetitive. This will be watchlist. Update this, and then migrate everything over. First up, we grabbed the series. Like so. Return that.
Like so. Return that. Next we need, the title is your watchlist, and the description. And then no card theme. Okay. So let's try once again, return new watchlist, run the code, it passes, remove all of this, and then yet again I can say, only if you're signed in do we return your watchlist. Run the tests, and everything passes. Great. So now, we're cleaning things up, and you're starting to see, well these method calls simply
Great. So now, we're cleaning things up, and you're starting to see, well these method calls simply return a class. And this is what we're shooting for. In almost every case, they just return a class. And further, if I scroll back up, look at the imports here. I can remove these, because we're no longer dependent upon Redis directly in the query class. So now we get this. Secondly, notice how much more information we have at this point.
Inline Classes, Remove Methods13:18
So now we get this. Secondly, notice how much more information we have at this point. Originally, we had some complicated Query class we didn't understand. But now I can see, alright, here are the specific featured collections we can reach for. Trending, recently updated, watchlist. This is what I want to see. Okay. So now, if we come back, if I have a method call that just returns a new class, let's see if I can remove that call entirely and just prepend new Trending. Run the code, it works.
Now I want to get back to where only a single instance is returned. That still works, which means I can now inline this. Run it. It works. And then update this one. New in progress. That works as well. So I can finally get rid of these methods. And this is where we currently are. So in fact, I'm just going to do this, get rid of all of that, run it, good.
