تماشای این درس نیاز به اشتراک حرفه‌ای دارد.

Being Strict With Controllers0:00

The next technique is to be strict with your controllers. So let's talk about what I mean by that. Here you can see a TeamsController. Now don't worry about the code too much, but do focus on the methods that I have here. This is all of them. Now you'll notice that often a controller, if we're building a RESTful controller, it will have the seven RESTful actions, but then we have all these compound words, or methods consisting of two words. Generally, there's nothing wrong with this, but often I find that it's a hint. When I have a method name that is multiple words, it's a hint that I might, not always,

Generally, there's nothing wrong with this, but often I find that it's a hint. When I have a method name that is multiple words, it's a hint that I might, not always, but I might be missing something. Something might want to be extracted. So let's use this example, storeTeam. We have a TeamsController. Why didn't we call the method store? Well, we didn't call it store because we also have one for storing a member. So if this is called store, is it storing the team or the member? It gets confusing.

So if this is called store, is it storing the team or the member? It gets confusing. So we chose to be explicit here, and we used multiple words, store team and store member. So already we've abandoned the RESTful actions. All right. So if we take a look at this, we can see this controller is responsible for Teams, right? However, yes, we can show a team and create a team and persist it, but then it looks like we have other functionality related to the members on the team. Add a member, remove a member, let a member voluntarily leave. And then finally, we have some protected or private methods that simply assist the actions.

Add a member, remove a member, let a member voluntarily leave. And then finally, we have some protected or private methods that simply assist the actions here. So we end up in a situation where a lot is going on. And from my experiences, when you take this approach, the next stop, once you feel things becoming gross and you have all these protected methods that often only assist the single action and no other, and you start to think, well, that's weird. We have this one protected method that's not really related to the controller. It just assists one action here. That's kind of weird.

Avoiding Service Class Creep1:59

It just assists one action here. That's kind of weird. So maybe I should create a Service class instead. So the next step, the next rung of the ladder is you extract a Service class and then maybe leave team or storeMember moves to the Service class. And then the related private method moves as well. And you pat yourself on the back for doing a good job in cleaning things up. And this is often what's recommended. And I've no doubt there are good use cases for that exact approach. Instead, let's be strict with our controllers.

Return to RESTful Actions2:26

And I've no doubt there are good use cases for that exact approach. Instead, let's be strict with our controllers. Let's make a rule that we're not going to abandon those seven restful actions. And if we find ourselves reaching for that and wanting to abandon it in order to create methods like these, instead, we'll simply create a new controller so that we can return to the seven restful actions. Let me show you what I mean. So I have a handful of tests backing up this Controller here. The first thing I see is if we have a Controller for Teams, then it should be the Controller for Teams.

The first thing I see is if we have a controller for Team, then it should be the controller for Team. It's not the controller for the users on the team. It's the controller for the team itself. So with that in mind, I'm going to visit these methods like this one, create Team. This will show a page to create the team. I want it to be create, returning to the seven restful actions. So store, what else do we have? storeMember, that's going to be extracted to its own new controller. Same with this one.

Store member, that's going to be extracted to its own new controller. Same with this one. We're not sure about that one yet. And then we have these random protected methods here. Okay. So immediately if I run my code, everything's going to blow up here. And that's because I've changed the method names. So I'm going to go to my routes file specifically for the teams. And I will quickly update these. So createTeam is now create.

And I will quickly update these. So createTeam is now create. storeTeam is now store. So if I run the code again, we're back to green. All right. Next, these methods that are half restful and then half some other noun, well, I keep seeing member everywhere. It sounds like that should be its own controller. Because if it is, I can then make this word redundant. I'll show you what I mean.

Extract Team Members Controller4:01

Because if it is, I can then make this word redundant. I'll show you what I mean. If we have TeamsController, how about we do TeamMembersController. And then the action no longer needs to be storeMember because it's redundant. TeamMembersController storeMember. No, we don't need that. So let's return it. Okay. Next, where else do we have one? This one as well.

Next, where else do we have one? This one as well. TeamMembersController at destroy. Next up we have leaveTeam. That's going to be a slightly different. So we'll deal with that a little bit later. All right. So now if I run my tests again, everything blows up because there is no TeamMembersController. But that's my next step.

controller. But that's my next step. php artisan make:controller called TeamMembersController. Okay. Now, if I run my tests again, we don't have any of those actions. All right. Let's visit that. And we know we need store. And I think we had destroy. Okay.

And I think we had destroy. Okay. Run the tests again. And now it's failing because we need to migrate over some of that controller code. All right. So right here, storeMember, again, the code here isn't overly relevant. We're validating the request. We tried to invite the member, but if there's any issue related to it, we just return some JSON. Otherwise, if it's not an Ajax request, we refresh the page and flash a message.

JSON. Otherwise, if it's not an Ajax request, we refresh the page and flash a message. Not overly interesting. So we'll grab that, switch over to our new controller, and paste it in. And now once again, because I've moved it to its own controller, I can return to store. Okay. All right. So next, let's switch back. It's noting that we have duplicate code here. Get rid of that.

It's noting that we have duplicate code here. Get rid of that. But next, you'll see certain things like inviteMember. All right. Well, this is a protected method where we try to track down an existing User. If there isn't one, they get invited. Otherwise, they are automatically added to the Team. So anyways, let's extract this to our TeamMembersController. All right. And next, these two methods are related to that.

All right. And next, these two methods are related to that. So those guys can now come right down here. And next, it looks like phpStorm wanted to make that static. Nope. And then we also need that destroy method. So if I come back, here we are. So here we are destroying the member. So we try to find the user, but if we couldn't, they were just invited. So cancel the invitation.

So we try to find the User, but if we couldn't, they were just invited. So cancel the invitation. Otherwise, remove the member and return a flash message. So yet again, we grab all of that and we move it to its own controller. And then yet again, we can return to one of the seven restful actions. Now we're getting close, but one thing here, getTeam, we don't have that method. So I will grab that on the original controller, put it in here at the bottom. So yeah, there is an argument to be made for cleaning up some of these protected methods. And we'll talk about that in a bit. But for now, we're just trying to get back to green.

And we'll talk about that in a bit. But for now, we're just trying to get back to green. So if I run this, we got to import team. Run it again. We're getting close. Find user by ID. Okay. It looks like we have a method here that's a little wonky and that we can very likely clean up. Run it again.

clean up. Run it again. And there we go. We're now back to green. Okay. So take a look. If we go back to TeamsController, scroll to the top, notice that we've returned to those familiar action names, show, create, store. Leave team still a straggler. That'll have to be dealt with as well.

Leave team still a straggler. That'll have to be dealt with as well. But other than that, we have a couple of protected methods that assist these actions and we're good to go. Now TeamMemberController, if we take a look, the same thing here. It's no longer storeMember or destroyMember. We've returned to those seven action names, store and destroy. Now here, I do see one thing though. inviteMember. This is where we find the User and we either invite them or add them immediately to the

Invite member. This is where we find the User and we either invite them or add them immediately to the team. Well, notice once again, I no longer need to say invite member because I'm on a TeamMemberController. So I can change that to a simple invite. All right. Are we still? Okay. Yes, we are.

Okay. Yes, we are. Okay. So now if we take a look, there we go. Notice single words here, store, destroy, invite, refresh. This is what I like to see. Now it is true in some situations like find User by email. Okay. No problem. We're mostly focusing on the core actions.

Removing Unneeded Helper Methods8:28

No problem. We're mostly focusing on the core actions. However, I do see findUserByEmail and findUserById. Do those really need to be on this controller? Aren't there, or in other words, aren't there other places in the project where you got to find a user by their ID? That just seems very verbose and unnecessary. For example, findUserByEmail. Is that really necessary? So this would be an example where I ask myself, is there any reason this belongs on the controller?

Is that really necessary? So this would be an example where I ask myself, is there any reason this belongs on the controller at all, or should I instead move it elsewhere? So let's find where I'm using this. Okay. So it looks like we only have one reference here. So the first thing I might do while I'm at green is find this method and just pull it out. All right. So bring it right back up and we'll say member equals that.

All right. So bring it right back up and we'll say member equals that. And then we can remove that section. Okay. So now if we scroll down, I can remove that method entirely and I've cleaned up my controller a little bit. Next if we run the tests, we're still good. But yeah, at this point you might extract a method. You might add a static method on User or you might just keep it simple like this. The world's not going to burn if you do an Eloquent query in your controller.

You might add a static method on User or you might just keep it simple like this. The world's not going to burn if you do an Eloquent query in your controller. And if we run it, we still get green, but we've cleaned up our controller. Okay. Next, let's do one more. We'll take a look at getTeam here. Here we are. Now in this case, we are finding a team. We're eager loading the manager of the team so that we don't have to perform another SQL query to find the User who owns the team.

Move Queries to Models10:03

We're eager loading the manager of the team so that we don't have to perform another SQL query to find the user who owns the team. And then we're scoping it. So give me the team owned by the currently signed in user. Okay. In this case, I'm using the getTeam method in a number of places around the controller. If that's the case, it seems silly to repeat this code in three or four different areas. But instead, what if I said team and then we made ownedBy not a query scope, but a simple static method that'll perform this query on my own. So we'd have something like that.

simple static method that'll perform this query on my own. So we'd have something like that. A little bit cleaner. And if I took that approach, I'm a little less inclined to have a getTeam method. Okay. So let's try this. I'm going to run the code and everything will blow up. Okay. So let's hunt down the ownedBy method and you'll see here are all the places in the project where I reach for it.

So let's hunt down the ownedBy method and you'll see here are all the places in the project where I reach for it. So first step, here's where it's defined on the Team class. And then we have a test for it. And then we have three uses in the controller. Okay. So I'm going to visit the ownedBy method here and you'll see again, it's a query scope. Now it looks like once again, when we search for it, every time I use it, I'm just finding the team owner. So it doesn't sound like it's serving much of a purpose as a query scope to help.

And then say first. Next though, don't forget, it sounds like we always want the manager. We could do it here or on the Team Eloquent model itself. We can use the with property to specify relationships that should no matter what be eager loaded. But in this case, we'll be explicit. Okay. So now I can say, find the Team owned by the given User. Okay. So now if we run it, it looks like we're still calling ownedBy as a query scope. Maybe let's go to TeamMembersController ownedBy now it must be on the parent controller.

So now if we run it, it looks like we're still calling ownedBy as a query scope. Maybe let's go to TeamMembersController ownedBy now it must be on the parent controller TeamsController. And there it is. So it looks like we have this method on both controllers. Another sign that it should be extracted. So I can say teamOwnedBy the authenticated user. And with any luck, we'll get green. And we do. Okay, great.

without needing a method like this. But that's for another lesson. So for now, I'll paste this in. And then we'll just inline it there, or actually inline that if you prefer. Let's keep it like that. Okay, so now I've made this method redundant. Run the tests. We're good. So now if we take a look, show, create, store, much cleaner, leave team, we're not going to cover that, but it's basically the exact same process.

So now if we take a look, show, create, store, much cleaner, leaveTeam, we're not going to cover that, but it's basically the exact same process. We're going to find a more appropriate controller, whether it exists or we have to create a new one. And then once we do, we will change leaveTeam to destroy. We're destroying the user's participation in a team. That's how we think of it. Finally, Owner has already created a team. Yeah, this would be a case I think there's something missing there. So if we take a look, we are tracking down the team and then we are just checking to

Yeah, this would be a case I think there's something missing there. So if we take a look, we are tracking down the team and then we are just checking to see if it exists. And that's okay, but you'll see we're only reaching for it in one place. So we're saying if the user has already created a team, then they don't need to see this create page. It's a one-time thing where you set up the team. So in that case, we'll just redirect them to their dashboard. But if we were to extract that, let's try this. Find the team.

But if we were to extract that, let's try this. Find the team. And again, I'm thinking we might want to change this to firstOrFail so that it throws a ModelNotFoundException that we can then handle maybe a bit more cleanly. But without that, we would just say, well, if there is a team, then redirect. So now, one last time, we're not reaching for this method anywhere else. And by the way, I'm not against methods that are only called once. If it allows you to take a chunk of code and assign a name to it, I think that's a win. But all of this comes down to sometimes you want it, sometimes you don't. So one last time we run our tests, we're at green.

But all of this comes down to sometimes you want it, sometimes you don't. So one last time we run our tests, we're at green. Here are the methods on this controller, a big improvement from before. And now for TeamMembersController, this is much better as well. So we had that getTeam where we got rid of this. Now, as it turns out, I do reach for getTeam a number of places here. So not a big deal to have a protected method there. Or you could even have a trait that you pull into the controller. Something I've used in the past is like ModelFinder or a little trait that will assist you with fetching common instances or collections that are used in multiple places for your

Something I've used in the past is like model finder or a little trait that will assist you with fetching common instances or collections that are used in multiple places for your project. That would be an option as well. But I don't think it's necessary here. All right, so at this point, there might be a couple of things we'd want to continue refactoring. But hopefully, the basic idea has been imprinted on you. If you ever have a controller with a growing number of actions and a growing number of protected methods, and it's very confusing, instead, ask yourself, is there a way that I can extract a new controller so that I can then return to those basic seven RESTful

protected methods, and it's very confusing, instead, ask yourself, is there a way that I can extract a new controller so that I can then return to those basic seven RESTful actions?

دوست دارید گاهی خبرهای Laracasts را ایمیل کنیم؟