|

How To Refactor Code To A Dedicated Class

How To Refactor With PHPStorm

In this tutorial we are going to learn all about how to refactor with PHP Storm. During the course of building our application, we often find the methods in our controllers begin to expand in size. As the need to apply more and more logic happens, we don’t want to see our methods get so convoluted that they become hard to read and reason about. In this tutorial, we’ll try our hand at refactoring some code to make it more pleasant to read and work with in the controller, while still maintaining the same functionality of the application.


Refactoring the ThreadsController@index() method

Here is the method we want to refactor.


Creating A Filtering Class

Our goal here is to create a new Filtering class that will do the heavy lifting for us, and then we just inject that class into our method or controller and call a simple method or methods as needed. So what we are going to do is create a new directory within the app directory named Filters. In the new Filters directory we will create a new file named ThreadFilters.php and start with an empty shell of a class like so:


Imagining Your API

Now we can begin to experiment with how we might like to interact with this class in our controller. Here is a beginning, and we can examine the idea:

What we want to do is inject a class into our method and then pass that to a query scope on the Thread model. The Thread model will then just intelligently apply the scopes and filters as needed, and give us the results we want.


Build The Supporting Code

In order to be able to call those methods above, we need the code behind them in place. First up, let’s add a filter() method to the Thread model. Go ahead and open up Thread.php and add the following method at the end of the class:

This is our query scope, where we accept the query and the filters. We then return the applied query from within the function. Ok, well – that apply() method does not yet exist on our ThreadFilters class, so we will need to create it. Open up ThreadFilters.php and set up this code:


Run tests after the refactor

As you go about refactoring your code, you want to run your tests every so often to see where things might be failing. At this point, all is passing, so this has been a good refactor so far.

vagrant@homestead:~/Code/forumio$ phpunit
PHPUnit 6.5.5 by Sebastian Bergmann and contributors.

.....................                                             21 / 21 (100%)

Time: 2.91 seconds, Memory: 12.00MB

OK (21 tests, 37 assertions)

Extract A Method

We can use PHP Storm to automatically extract a method for us. What we’ll do with this refactor is to take some of the code within the apply() method and extract it out to it’s own method. In PHP Storm, you can highlight the code you would like to extract to a method and right click. In the sub menu that pops up you choose Refactor->Extract->Method.
phpstorm refactor extract method

At this point, you now get a chance to name your method and choose the visibility of it. We will name our extracted method ‘by’ and make the visibility set to protected.
phpstorm name method choose visibility

Here is the generated code form PHP Storm highlighted below. This is a really nice feature of the PHP Storm IDE as it does some of the heavy lifting for you when refactoring.


Assigning $builder to the object

Let’s keep refactoring. In both the apply() and by() methods, $builder is getting sent through, but we could assign it to the object instead which might make it just a touch more readable. Once again we can use PHP Storm to help us with this. Here we assign $builder to the object, and then click on builder. At that point, if you click on alt + Enter in PHP Storm – you can add the field to the class which we also complete here.
assign field to the object

After the $builder is assigned to the object, our code can be updated like so. The changes we notice are as follows.

  • $builder is now a protected field of the class.
  • $builder itself is no longer passed as an argument to the by() method.
  • In the by() method we now reference builder using $this->builder

The highlighted lines show where this refactor has an impact.


Checking if the request has ‘by’

Now we will add some logic to the apply() method to check to see if the request being made contains the ‘by’ in the query string. If it is in the query string, then we will apply that filter.


How To Use An Abstract Parent Class

Now we’re really going to blow your socks off. We will make use of an Abstract Class to remove code that could be reused in multiple scenarios as well as make the child class more lightweight. We can put the repetitive stuff up in the abstract class. So within the Filters directory, let’s use PHP Storm to create a new PHP Class.
phpstorm create new php class

You will need to manually add the abstract keyword, but here we go with our new abstract Filters class.


Using PHP Storm To Pull Members Up

Now that we have an abstract parent class (Filters) we can visit the sub class which in this case will be ThreadFilters.php and make it extend it’s parent class. Once you define the class definition to ThreadFilters extends Filters, you can click on the request field and bring up the ‘refactor this’ menu. In the Windows version this is done by clicking Ctrl+Alt+Shift+T. If that is too much to remember, right click and choose Refactor->Pull Members Up.
php storm refactor this

Selecting the members to pull up.
selecting members to pull up


PHP Storm Automatically Builds the Abstract class for you

Now that you have used PHP Storm to pull members up for you, all of that reusable code has automatically been transferred to the parent abstract class of Filters.php, while the sub class of ThreadFilters.php has gotten cleaned up considerably.

Here is the nice and clean ThreadFilters class.

And here is the Abstract Parent Class which PHP Storm essentially built for you.


Further Refactoring In The Abstract Class

First off, we will add a field in the ThreadFilters class which is an array. That array will hold various strings of what we want to filter by. The parent class will also need a field which defines an empty array.

Now comes the somewhat trickier part. We want to remove the hardcoded ‘by’ filter from the apply() method. We now want the ability to choose from an array of filter types. So what we set up is a foreach loop where we loop over each possible filter, and if there is a method for that filter, trigger it with the value from the filter. The highlighted code is here to reflect that.

This is looking pretty good, and the tests are passing. What else can be refactored? Well, the code inside the if conditional is kind of fugly so let’s extract that to a protected method in the class. Go ahead and make use of PHP Storm again to do the work for you.
refactor to protected method in phpstorm

Check out your nicely extracted method now! Within the apply() method we make a simple call to hasFilter() and everything still works. The heavy lifting is moved down to that protected method PHP Storm created for us and highlighted below.


Shorten your if() block with a return statement

Here is another refactor you could do. You can turn the if statement you see above into something like this:


Protecting Yourself With only()

As you are likely aware, internet users are going to try to hack query strings and send bad data to your application. So how do you protect yourself from that? Well right now, let’s see how this works. We are going to add a dd() statement to the apply() method like this:

Now we can test query strings in the browser. If we visit http://forum.io/threads?by=asdf in the browser, we see the filter array populate as we expected.
good query string

What if we were fetching the request with the all() method in this manner $this->request->all() and a user tries a hacking attack by visiting something like this http://forum.io/threads?by=asdf&hacker=attack Well in that case, we would have a problem.
bad query string

At that point, you’ll be accepting that bad data right into your application. We see that evidence above, and that is something we want to avoid.

With our new knowledge about the only() method, let’s make use of it in our foreach loop on the apply() method. Also note that since we are looping over an array of key value pairs, we also need to include $value in the foreach() as we see here.

Let’s also do another refactor to clean up that code inside the if conditional.
refactor this method

Naming the extracted method to getFilters()
extract method signature preview


Removing the hasFilter() method

As we have gone along refactoring, it looks like we could actually do away with the hasFilter() method now. We will just update the apply() method to handle this. Basically now, as we loop over the filters, we check to see if a method exists on the object with the same name as the filter, and if so trigger is while passing through the filter.

With that factor, we can now delete this snippet from the class.


A final refactor on the ThreadsController@index method

Recall, all this refactoring started because we wanted to make the index() method more streamlined in the ThreadsController. Let’s revisit that method one more time, and just clean it up a bit more.

Here is the current state of the method

And here is a more streamlined refactor of the same code

The last thing we will do is to go even further, and extract one more method out of this code so that our final index() method is as simple as can be.
refactor this option 7

Name the extracted method
refactor method protected parameter


The Final Result

Whew. I feel like my brain is going to explode. In any event, this was the long version of how you can use refactoring to go from this code

All the way down to this code here


How To Refactor Code To A Dedicated Class Summary

It takes some time, and a lot of test running as you go. To be fair, phpunit was being run in the background almost constantly during this entire tutorial. We skipped some of that just for brevity. The take away here is that if you like to keep your controller code clean, you will need to be able to lean on the tests you have created and keep refactoring until you have an API that you like while continuing to keep all of your tests in a passing state. Once both of those are in check, then your refactor is complete!