Advanced OOP For WordPress Part 7: Refactoring Is An Opportunity To Adopt Test-Driven Development

So far in this series on writing WordPress plugins using object-oriented PHP, I’ve focused mainly on testing. This is because a key advantage of object-oriented PHP is that it can be written in a way that is both highly testable and highly reusable. Just using classes doesn’t magically give us this

We’ve also not written any code that does something practically useful. The idea is to show how to customize search via the WordPress REST API. The example plugin, at the point where I’ve left off just returns an array of WP_Posts. But, the public API for the system is now fully developed and has tests describing its correct behavior.

In my personal journey as a developer, I’ve taken testing more serious and less seriously at times. Right now, I’m in “TEST ALL THE THINGS!!!” mode and one of the reasons I love it is the freedom to refactor. With untested code, it’s hard to refactor as you do not know what the side-effects will be.

While writing testable code reduces possible side-effects, test-driven development (TDD), helps you know what the effects of a change are. Moving to TDD is not easy, but I’ve found it can be adopted incrementally in existing codebases. So, this article begins a part of this ongoing series focused on test-driven refactoring.

Code Review Before Moving Forward

When I started this series, Tonya Mork of Know The Code reached out and offered a code review of the first article. She’s been writing code review articles, that I hope you are enjoying as much as I am.

This article’s example code really starts at the pull request that Tonya added to go along with her article Designing Swappable Search Request Implementations. I agreed with her review and accepted the changes, and then we discussed further changes. The comments on that pull request are essentially a rough draft of this article and the next one in this series.

You can read through our process of making a lot of changes to the code and why. For me, I find that writing everything I know about a problem, and answering questions about it is incredibly helpful. That’s code review. I strongly recommend you introduce it at your company and/ or amongst your friends.

What’s The Intent?

One thing about the example code we’re working with is that it started fairly contrived, it’s example code after all. But, we’re slowly making it actually useful, because that is more fun. As a result, the interface that Tonya defined for objects that would act as our “content getter” — the system that actually got content from the database or API, didn’t ask the right questions of the outside world.

Tonya summed up our conversation here, we decided to pass around the WP_Query and the WP_REST_Request, as those objects expressed the intent of the HTTP request in order to fulfill the intent of our code — we want queries to post type routes such as/wp/v2/posts to be more useful for search on some post types. By more useful we mean different types of WP_Query or results generated with a different system.

Planning Interfaces As Glue

I went into this thinking my goal was to have one factory that wires everything and demonstrate how to use tests to prove that I didn’t break anything along the way. That involved creating interfaces for all of my objects, so I could create testable factories and then updating the existing interface, so it all could be wired together.

To be honest, this is where I struggle often. I’ll have two or three systems that work in isolation and then will have trouble putting them together. Adding new interfaces for ModifySchema which changes the endpoint’s schema and ModifyQueryArgs which changes the allowed WP_Query args was my first step to gluing everything together. Starting there allowed me to define the shape of how they would connect later.

Neither of those classes had an interface. They both have a shouldFilter() method, but they have different signatures. FiltersPreWPQuery also had a shouldFilter(), different signature, and it is static. This smelled bad to me.

Given that I needed to wire an arbitrary number of implementations, I assumed we would need to hook rest_{$post_type}_collection_params and rest_{$post_type}_query for every post type. I actually got this idea when I was searching my local WordPress environment for those hooks. I was trying to find those hooks in the REST API and found where Gutenberg uses them instead Gutenberg hooks into both of the filters we need to work with rest_{$post_type}_collection_params and rest_{$post_type}_query here.

View the code on Gist.

My goal was to make the filtering as magic as possible since that’s repetitive. I actually began with an interface that required it, and then removed that requirement as I implemented a factory to handle more of the gluing together.

Let’s start with refactoring the class that models the REST API schema. The first step is to add an interface. By doing that we will be prepared to transform this class from the class that modifies REST API route schemas, and turn it into a swappable implementation of a class that modifies REST API route schemas.

Designing an interface before a concrete implementation, if possible may prevent having to do the kind of refactoring I’m about to walk through. But it is often more practical to build the first implementation without the constraints of an interface and then once it works, design the interface. Sometimes when I start with an interface, I’ll find myself struggling to conform my concrete implementation to an interface and put too much time trying to make it work before I realize the contract I set out was wrong.

Often times we refer to interfaces as “contracts” because they define how a class is required to act in relation to the rest of the program. Once we commit to the contract, we’re stuck with it. Committing to a contract that sets us up to fail is bad in business and in programming.

My first step when creating an interface for an existing class is to add an empty interface and have the original class implement it:

After that, I can copy the method signatures and inline docs to the interface. An IDE that is aware of interfaces, I use phpStorm helps a lot in this process, as it will highlight method signature conflicts as they arise.

Here is the ModifySchema before I began refactoring:

View the code on Gist.

I said earlier, my goal was to make the filterSchema method unneeded. While my first  thought was to make the interface have filterSchema() and shouldFilter() methods, beacuse that’s what currently needed to be exposed publicly. But I wanted to make <code>filterSchema</code> go away, so I thought about what information this class needs to communicate:

  • The additional schema arguments
  • Whether or not the additional schema arguments should be added.

Based on that list, shouldFilter() makes sense, but filterSchema() does not. It also says the first step is to abstract out the array of arguments to add and make them publicly accessible. Here is the beginning of the interface:

View the code on Gist.

This new method getAdditionalSchemaArguments() is type-hinted to return an array. We can reliably use it later to get an array of schema arguments. The original class doesn’t have getAdditionalSchemaArguments(). It has an array of arguments, but they are currently defined inside of filterSchema(), which is not what filterSchema() should be responsible for, its a 2nd responsibility.

That smells bad. Let’s fix that by ensuring that each method does one thing — the thing the name of the method says it does.

View the code on Gist.

In this new version we still have filterSchema() but all it does is handle filtering the schema. The data is provided by the new getAdditionalSchemaArguments(). All three methods have had there inline docs replace with an @inheritdoc annotation. That’s because they are all present in the interface.

View the code on Gist.

Recounting the story now, it’s debatable if it was worth adding filterSchema() to the interface, knowing I was going to remove it was worth it. I had to refactor my test mocks to conform to this interface as well. Once an interface is used in the real world, changing it is a backwards compatibility breaking change. Interface signature changes happens in Laravel major version upgrades. You have to change your concrete implementations to match or the upgrade causes fatal errors.

In our case, none of this is production code. Not just because it’s educational code, but because its not done yet. We’re going to need to make a few changes along the way and it makes a good story anyway. Here is the commit where I created this interface.

We also have a class ModifyQueryArgs that is responsible for adding WP_Query arguments to the REST API’s whitelist of allowed query arguments.

Here it is, implemented to add a “post_type” argument to the allowed list:

View the code on Gist.

With this class, I smell the same issue as the last one, it’s not communicating what it will modify the whitelist with. Before we can make the filterQueryArgs() method go away, we need a method to communicate what is added on the filter. This time, a getAdditionalQueryArguments() method can communicate that:

View the code on Gist.

My first step of refactoring the original class was to make it match this interface, which for now maintains
filterQueryArgs().

View the code on Gist.

That’s the second interface we need to start fitting this together. You can see the commit for the new interface here.

Making The Pieces Fit

Tonya and I both use the metaphor “gluing it together” to describe this process. Maybe I’ve just watched too many pieces of furniture be put together recently because I just moved, but this process feels more like shaping the ends of a board so they will slot into another one perfectly.

I just walked through the step of making the board fit like I think it will need to fit. The next step is a factory that it should fit correctly with. I did my best to imagine what that needs, once

While we’re playing with metaphors, it’s worth mentioning that one of the benefits of Laravel is that its service container provides automatic dependency injection to autowire dependencies to classes. For example, in WordPress, if I have a REST API route, that needs to access a user, I use get_user_by() or new a WP_User, inside of the callback method. In Laravel, I would just make the user model an argument of the controller callback and based on its typehint, Laravel would handle injecting a user model to my callback.

By transferring this responsibility for providing the dependency to the service container — we call this inversion of control — makes swapping concrete implementations, for example using mocks for testing much simpler.

Tonya has a really good series on mocking objects in WordPress. Mocking is a very useful skill. I find it to be the slowest part of writing tests and find that when I write tests for Laravel or React, mocking comes more natural and involves less mocks.

I’m not saying all of this to call out WordPress. On the contrary, I’m going on this tangent to explain the upsides of the approach we are teaching here. You can have these things in your PHP and JavaScript development. The code in this article and the next article was added to the example plugin via this pull request. In my next post, I’ll continue showing how to use tests to help refactor and better glue together the system.

Josh is a WordPress developer and educator. He is the founder of Caldera Labs, makers of awesome WordPress tools including Caldera Forms — a drag and drop, responsive WordPress form builder.

The post Advanced OOP For WordPress Part 7: Refactoring Is An Opportunity To Adopt Test-Driven Development appeared first on Torque.