Latest News

Refactoring and Code Reviews

I’ve been reading Refactoring: 2nd Edition by Martin Fowler as part of the Dev Book Club for the past three months. If you’re unfamiliar with the book, it covers a variety of techniques you can use to make changes to existing code that ultimately makes the code function exactly the same as it did before, but which should (hopefully) leave the code easier to read and understand for you and anyone else who needs to read it.

The 2nd Edition examples are all written in JavaScript, compared with the original book (published in 1999) which instead used Java. This hopefully makes it a little easier to digest for folks who do most of their programming for the web, and particularly for those who are coding primarily with JavaScript itself, which seems to be an ever-growing number of developers these days.

If you do any work developing applications with other people, then you’re probably familiar with the concept of a code review. Code reviews are an opportunity for other developers to look at the code you’ve written and to provide feedback around various aspects of your proposed changes. When providing feedback to others during a code review, outside of confirming that the functionality of the changes works as expected, I generally ask myself a few questions:

  • Do I understand what this function or block of code is doing?
  • Do I think this code will still be clear 1, 3, 6, or 12 months from now?
  • Is this code broken down into small enough parts (e.g., is it only doing one thing, and doing it well)?

A variation of these questions is “Are there opportunities to make this easier to understand?” If there are, then I look to make those recommendations.

This morning, I was poking around in completely random parts of the WordPress Core codebase for no particular reason, other than I was thinking about this Refactoring book, and I wanted to see if there was anything that would jump out at me. WordPress as a project doesn’t really do a lot of refactoring on the code base over time, only lightly changing parts of existing code if it’s specific to a bug or feature (which, as an aside, I think is an extremely good policy to have, and in general, I’m really glad that the contributors to the project do this). Looking at the WP_Widget class, something that’s existed in WordPress since version 2.8, I noticed one method that piqued my interest and led me to write this blog post – the get_settings method. Let’s take a look at it:


/**
 * Retrieves the settings for all instances of the widget class.
 *
 * @since 2.8.0
 *
 * @return array Multi-dimensional array of widget instance settings.
 */
public function get_settings() {

	$settings = get_option( $this->option_name );

	if ( false === $settings ) {
		if ( isset( $this->alt_option_name ) ) {
			$settings = get_option( $this->alt_option_name );
		} else {
			// Save an option so it can be autoloaded next time.
			$this->save_settings( array() );
		}
	}

	if ( ! is_array( $settings ) && ! ( $settings instanceof ArrayObject || $settings instanceof ArrayIterator ) ) {
		$settings = array();
	}

	if ( ! empty( $settings ) && ! isset( $settings['_multiwidget'] ) ) {
		// Old format, convert if single widget.
		$settings = wp_convert_widget_settings( $this->id_base, $this->option_name, $settings );
	}

	unset( $settings['_multiwidget'], $settings['__i__'] );
	return $settings;
}

Thinking through my short list of code review questions, and with the Refactoring book top-of-mind, I examined this method with a somewhat puzzled look. Take a look at the above again and see which questions come to mind for you – I’ll wait.

Ready?

For me, there were a few things here that caught my eye:

  1. We (of which I mean the WordPress community, because anyone can contribute to this project) mix terminology in here, “settings” and “options”.
  2. The block of code that checks if settings are false takes a bit of mental overhead to understand what it’s doing, in part because it’s checking for an alternate option (née “setting”) name for the get operation, and possibly saving the setting (née option) if it doesn’t exist, which is completely outside the responsibility of this function.
  3. The $settings variable potentially gets reassigned at least three times, making it harder to hold in my mind what values it might contain at any given step in the runtime.
  4. There are some complex conditional statements that might be more easily described with a method name.
  5. There is removal of some array keys every time, even if it may not be necessary, and it’s not entirely clear why they’re being removed.
  6. The single return point at the end of the method adds to the mental overhead of knowing when particular conditions are triggered.
  7. There are comments denoting information about what the code is doing that might be better served with a function that says what it does.

There are likely some other cases here to examine, but even six observations within a single 24-line block of code is quite a bit. This single method is doing quite a bit of work, even if its main job is to simply get the settings for a widget. In plain language, it’s asking the following questions:

  1. Have I saved options using my primary name or secondary name?
  2. Do I need to save options because they haven’t already been initialized?
  3. Are my options saved in a format I expect?
  4. Do I need to convert my options?

These are clear indicators that a function is likely doing too much. Additionally, because there are some questions about why some parts of the code exist, it means changing it to become easier to read has the potential to be fragile, because there’s some context here that maybe only the original programmer who wrote this function understands. In Refactoring, Fowler indicates that his first step is to write tests for code if the tests don’t already exist, and then running the tests in each step. This post won’t go into those details, in particular because of the complexity of getting unit tests up and running with WordPress, but I do want to cover the above questions and discuss suggestions for how this might get refactored to improve readability and reduce the number of questions about a function.

Let’s start with the last half of the function:

if ( ! is_array( $settings ) && ! ( $settings instanceof ArrayObject || $settings instanceof ArrayIterator ) ) {
	$settings = array();
}

if ( ! empty( $settings ) && ! isset( $settings['_multiwidget'] ) ) {
	// Old format, convert if single widget.
	$settings = wp_convert_widget_settings( $this->id_base, $this->option_name, $settings );
}

unset( $settings['_multiwidget'], $settings['__i__'] );
return $settings;

The first condition is sticking out to me, because it sets the $settings variable to an empty array in the cases where $settings is not already an array and it’s neither an instance of the ArrayObject or ArrayIterator classes. Basically, if it can’t be looped over, we want $settings to be an array here.

If that case is actually true, and $settings becomes an empty array, none of the rest of the function applies: since it’s empty, it won’t get converted, and there are no keys to unset. So why not simply return? We’re done, after all.

/**
 * Retrieves the settings for all instances of the widget class.
 *
 * @since 2.8.0
 *
 * @return array Multi-dimensional array of widget instance settings.
 */
public function get_settings() {

	$settings = get_option( $this->option_name );

	if ( false === $settings ) {
		if ( isset( $this->alt_option_name ) ) {
			$settings = get_option( $this->alt_option_name );
		} else {
			// Save an option so it can be autoloaded next time.
			$this->save_settings( array() );
		}
	}

	if ( ! is_array( $settings ) && ! ( $settings instanceof ArrayObject || $settings instanceof ArrayIterator ) ) {
		return array();
	}

	if ( ! empty( $settings ) && ! isset( $settings['_multiwidget'] ) ) {
		// Old format, convert if single widget.
		$settings = wp_convert_widget_settings( $this->id_base, $this->option_name, $settings );
	}

	unset( $settings['_multiwidget'], $settings['__i__'] );
	return $settings;
}

That is a subtle change, but it already significantly improves things in a couple of ways:

  1. $settings is not reassigned again, so we don’t have to think about what it holds anymore.
  2. Because we return early, we know that the rest of the function does not run in that case, so we can put it out of mind.

What if we took that a step further, then, and helped provide some context for the why we would return early in that case? Our complex condition is checking that we have something that we can loop over, so perhaps we could say that in a new method?

private function is_not_iterable( $settings ) {
    return ! is_array( $settings ) && ! ( $settings instanceof ArrayObject || $settings instanceof ArrayIterator );
}

Here, we make a new function that says what we’re actually checking, instead of leaving future readers of the code left to understand what we mean by the complex condition. Perhaps they’ve never used an ArrayObject or ArrayIterator class before, but they know what iteration is. So, if they’re reading get_settings to understand how it works, they might not need to look into the is_not_iterable function to understand what’s happening in get_settings. This is a clear separation of concerns, and leaves us with smaller methods to help make our functions a little more readable and with fewer responsibilities.

This does give us two methods. We’ll probably have more by the end of this exercise:

/**
 * Retrieves the settings for all instances of the widget class.
 *
 * @since 2.8.0
 *
 * @return array Multi-dimensional array of widget instance settings.
 */
public function get_settings() {

	$settings = get_option( $this->option_name );

	if ( false === $settings ) {
		if ( isset( $this->alt_option_name ) ) {
			$settings = get_option( $this->alt_option_name );
		} else {
			// Save an option so it can be autoloaded next time.
			$this->save_settings( array() );
		}
	}

	if ( $this->is_not_iterable( $settings ) ) {
		return array();
	}

	if ( ! empty( $settings ) && ! isset( $settings['_multiwidget'] ) ) {
		// Old format, convert if single widget.
		$settings = wp_convert_widget_settings( $this->id_base, $this->option_name, $settings );
	}

	unset( $settings['_multiwidget'], $settings['__i__'] );
	return $settings;
}	

private function is_not_iterable( $settings ) {
   	return ! is_array( $settings ) && ! ( $settings instanceof ArrayObject || $settings instanceof ArrayIterator );
}

Of course, it’s not a good practice to create methods that indicate they’re not something. That’s what the logical not operator is for. But to keep things working that way, we need to invert the logic and rename our method. Thus:

private function is_not_iterable( $settings ) {
    return ! is_array( $settings ) && ! ( $settings instanceof ArrayObject || $settings instanceof ArrayIterator );
}

Becomes:

private function is_iterable( $settings ) {
    	return is_array( $settings ) || $settings instanceof ArrayObject || $settings instanceof ArrayIterator;
}

Because arrays, ArrayObjects, and ArrayIterators can be looped over. Then we update our caller in get_settings:

if ( ! $this->is_iterable( $settings ) ) {
	return array();
}

This is already the smallest of changes, but now we’ve clarified some of the questions we had around variable reassignment and complex conditions, so hopefully get_settings is already easier to read than before. Next, I’m finding myself interested in the complex conditional block at the start of the method:

$settings = get_option( $this->option_name );

if ( false === $settings ) {
	if ( isset( $this->alt_option_name ) ) {
		$settings = get_option( $this->alt_option_name );
	} else {
		// Save an option so it can be autoloaded next time.
		$this->save_settings( array() );
	}
}

The main thing this block of code is trying to do is to get settings that are saved from the database. It’s potentially using two separate queries via get_option (checking alt_option_name if the first query failed), and it’s also saving the settings if both queries failed. The responsibility of the get_settings method is to retrieve them, so already save_settings feels out-of-place. But, of course, this is a long-standing method in an open-source application so it’s also possible it’s going to have to stay here. That said, I think there are opportunities to clean things up and clarify the responsibilities a little bit, and also potentially remove the duplicate variable assignment. Here’s one idea:

$settings = get_option( $this->option_name, get_option( $this->alt_option_name ) );

if ( false === $settings ) {
	// Save an option so it can be autoloaded next time.
	$this->save_settings( array() );
}

The get_option function in WordPress accepts a second parameter which is the default value if the value cannot be returned. $alt_option_name is null by default on the WP_Widget class, so we can in fact simply try to call get_option on the $alt_option_name property if the first call fails. get_option itself doesn’t actually make a database query if the passed in option is empty, so the performance implications here are small. The benefits here are that we avoid reassigning $settings if it was false the first time, and we get to remove the nested conditional check inside our first conditional. In theory, this is easier to read. Here’s our method now:

/**
 * Retrieves the settings for all instances of the widget class.
 *
 * @since 2.8.0
 *
 * @return array Multi-dimensional array of widget instance settings.
 */
public function get_settings() {

	$settings = get_option( $this->option_name, get_option( $this->alt_option_name ) );

	if ( false === $settings ) {
		// Save an option so it can be autoloaded next time.
		$this->save_settings( array() );
	}

	if ( ! $this->is_iterable( $settings ) ) {
		return array();
	}

	if ( ! empty( $settings ) && ! isset( $settings['_multiwidget'] ) ) {
		// Old format, convert if single widget.
		$settings = wp_convert_widget_settings( $this->id_base, $this->option_name, $settings );
	}

	unset( $settings['_multiwidget'], $settings['__i__'] );
	return $settings;
}

The next thing that sticks out to me is our two comparisons right after we assign settings. We know now that if we don’t have settings saved in the database that get_option will return false. We also know that false is not iterable. Even though I’d like to get rid of the call to save_settings altogether right now, it’s probably simply best to keep it for the moment, but we can slide that statement into the is_iterable check:

/**
 * Retrieves the settings for all instances of the widget class.
 *
 * @since 2.8.0
 *
 * @return array Multi-dimensional array of widget instance settings.
 */
public function get_settings() {

	$settings = get_option( $this->option_name, get_option( $this->alt_option_name ) );

	if ( ! $this->is_iterable( $settings ) ) {
		// Save an option so it can be autoloaded next time.
		$this->save_settings( array() );
		return array();
	}

	if ( ! empty( $settings ) && ! isset( $settings['_multiwidget'] ) ) {
		// Old format, convert if single widget.
		$settings = wp_convert_widget_settings( $this->id_base, $this->option_name, $settings );
	}

	unset( $settings['_multiwidget'], $settings['__i__'] );
	return $settings;
}	

Now that we’re down to two conditions, I’m noticing that the second condition performs the conversion if $settings is not empty. The rest of the function, in fact, performs logic with the assumption that settings is not empty, so it seems like something we should include in our early return. For now, let’s slide that condition up to the early return. We have a side-effect of saving an empty setting every time, even if it’s empty, but it’s worth the change to make the rest of the method readable for now:

if ( ! $this->is_iterable( $settings ) || empty( $settings ) ) {
	// Save an option so it can be autoloaded next time.
	$this->save_settings( array() );
	return array();
}

if ( ! isset( $settings['_multiwidget'] ) ) {
	// Old format, convert if single widget.
	$settings = wp_convert_widget_settings( $this->id_base, $this->option_name, $settings );
}

Now we know we only convert the widget settings if the _multiwidget index is not set.

This is now the state of the refactor:

/**
 * Retrieves the settings for all instances of the widget class.
 *
 * @since 2.8.0
 *
 * @return array Multi-dimensional array of widget instance settings.
 */
public function get_settings() {

	$settings = get_option( $this->option_name, get_option( $this->alt_option_name ) );

	if ( ! $this->is_iterable( $settings ) || empty( $settings ) ) {
		// Save an option so it can be autoloaded next time.
		$this->save_settings( array() );
		return array();
	}

	if ( ! isset( $settings['_multiwidget'] ) ) {
		// Old format, convert if single widget.
		$settings = wp_convert_widget_settings( $this->id_base, $this->option_name, $settings );
	}

	unset( $settings['_multiwidget'], $settings['__i__'] );
	return $settings;
}	

private function is_iterable( $settings ) {
    	return is_array( $settings ) || $settings instanceof ArrayObject || $settings instanceof ArrayIterator;
}

Clearly, there are still unanswered questions and additional opportunities to continue to refine this, but I think the code changes result in some general improvements:

  1. We’re down to only one reassignment of $settings.
  2. We return an empty array in cases where widget settings could not be located or when they were already empty.
  3. Failing cases return early, avoiding any additional processing.
  4. We separated complex conditional logic into a named method that (hopefully) more clearly defines the purpose of the conditional check.

Of course, it’s possible this change raises questions, as well, and it’s possible the logic may have some performance considerations that would require additional refactoring:

  1. Does it seem weird to call get_option as the default to the first get_option call? If not, perhaps this could be extracted into another private method with simple if conditions, returning settings after the first call if it’s not false, then returning false if $alt_option_name is not set, then returning the call to get_option on the $alt_option array property.
  2. I don’t love that we’re calling save_settings on an empty array, but I also don’t love that save_settings is inside the get_settings method to begin with.
  3. It’s still not clear why _multiwidget and __i__ indices on the $settings array need to get unset here. That parsing, again, seems outside the scope of actually getting the widget settings, but clearly it’s an important part of the process of getting the settings for a widget. But, because that behavior isn’t described and it doesn’t have anything to do with getting the settings, refactoring it to be done elsewhere seems like a good idea.
  4. Clearly, I would need to add documentation to the new is_iterable method, and any other methods that get extracted from logic elsewhere. That’s not the point of this exercise, but feel it’s important to call that out. :)

Summary

First, I’d like to encourage you to check out the Refactoring book if you haven’t read it before. This sample exercise on a single method within WordPress core hopefully demonstrates how going through the process of altering code with the goal of improving readability is an important and viable part of the development process. This post just touches on a couple of approaches that Fowler describes in his book, and it’s a worthwhile read for anyone who wants to improve the quality of their code.

Second, I hope this post helps you think about some new questions you may not have considered when writing your own code or reviewing the code of your friends and colleagues. Structuring code into a working state can often be a big enough hurdle, but going the extra mile to make it so that it’s easily understood by you and others today and down the road pays huge dividends in terms of maintainability.

Just Checking In

Blink your eyes in a pandemic and the next thing you know, three months have passed.

I really felt like I was getting in the groove for awhile, too. Last time I checked in, I wrote about how it’s 2021 and we should all really consider using namespaces in WordPress projects by now, and I was planning out my next set of topics for my series about Rethinking PHP devlopment in WordPress (is it a rule that I link to this in every post I write now?).

In fact, that series is still ongoing, and I’ve got a doozy planned for using objects as a data structure instead of arrays, but I’ve been starting and not being terribly good at finishing a few different side projects, and have also gotten a bit sidetracked by participating in the Dev Book Club, where we’ve been reading and discussing the 2nd edition of the great Martin Fowler book, Refactoring. I’ve owned the 1st edition for a long time and admittedly read about half of it, so it’s great going through it again because all of the examples are written in JavaScript instead of Java and it’s just really hitting me a bit better now that I have more developer experience under my belt than when I first tried readin git.

Since you were last here, too, I refreshed the theme on my site, and I’ve been digging into how to develop board games on the Board Game Arena platform, because even though I can see the light at the end of the tunnel in this pandemic and might be able to eventual play games with my friends in person again at some point this year, it’s still cool to learn new things and I’m hopeful that we’ll all continue to play games online in the meantime, too.

All of which is to say, I just wanted to write a little bit to say hey, encourage you to check out Gravity Forms 2.5 that finally released last week, and let you know that it’s finally springtime in Minnesota. I pumped air in my bike tires and took the briefest of rides a couple of weeks ago, and let me tell you: sheltering in place for like 15 months has really put a damper on my fitness. I’m looking forward to setting some new goals, getting out a little bit more this year, working on some fun developer tools, absorbing as much knowledge as I can about my craft, learning more about the world, giving back to my community, and figuring out just how to be as “normal” as possible again in the coming weeks and months.

In the meantime, hey! I hope you’ve been well.

WordPress and the Curious Case of PHP Namespaces

For part two of my series on Rethinking PHP Development in WordPress, I wanted to write about three significant elements of object oriented programming: classes, interfaces, and namespaces. A big reason I wanted to write about them together is because I think they’re so integral to understanding the way PHP developers who cut their teeth on WordPress learn their craft. However, I felt that might get a bit verbose and keep me from hitting that “Publish” button, so I wanted to focus on just one segment of that topic today: namespaces.

What is a namespace?

Before we can talk about namespaces, let’s focus on a second on symbols. In computer programming, a symbol is simply something that has a human-readable form. A function is a symbol. Class names are symbols. So are variables. We use symbols in programming because they help us understand what we’re trying to make our programs do.

We have control over what we name our symbols, the same way that our parents named us, and we name our own pets, children, and computer hardware (you’ve named your computer, right?).

In real-life, symbols can be complicated. Have you ever worked somewhere that had more than one Chris, Travis, Sam, or (C/K)arl (I do!). We differentiate between first names by passing down our family names, but sometimes, that’s not even enough. In the U.S., we have Social Security Numbers so we can differentiate – on paper, at least – between the thousands of John Smiths and Jane Jacksons in our country.

Last names and identifying numbers are a type of namespace. Basically, a namespace is a way to ensure that something that must be unique in your system is, in fact, unique.

In PHP, we have to ensure that all of our symbols are unique in order to prevent fatal errors. And, in the WordPress community, at least, we’ve established a couple of different ways of attempting to achieve that goal.

Function prefixes

Everyone who has written WordPress code for any length of time has been told time and time again to “prefix their functions.” What does that mean? Well, let’s say I had a function that registered a product post type:

<?php
function register_post_type() {
    register_post_type( 'product', array( /* My options... */ ) );
}
add_action( 'init', 'register_post_type' );

If you’ve been writing PHP code in WordPress for any length of time, hopefully the problem is evident to you right away, because in this particular example, I’m declaring a function name that is named the exact same as the WordPress core function I’m calling within it. This is called a naming collision, and it PHP, it will generate a fatal error, which causes your code not to complete execution.

They way we’ve gotten around this for years in the WordPress ecosystem is two-fold. First, we prefix our function calls to reduce the chance that someone else (usually another plugin) has declared a function with the same name as us. And second, we wrap that function declaration in some code that checks whether that function has already been declared. Here’s another example:

<?php
if ( ! function_exists( 'jmw_register_post_type' ) {
    function jmw_register_post_type() {
        register_post_type( 'product', array( /* My options... */ ) );
    }
    add_action( 'init', 'jmw_register_post_type' );
}

The function_exists method in PHP allows us to check whether someone else has created a function with the same name before we make a declaration for our own. The jmw prefix in this example attempts to further minimize the chance that someone has created another function with the same name.

This approach is perfectly suitable, and has worked reasonably well for many developers over the history of the WordPress project. If you examine the code from the WordPress plugin repository, chances are high that you’ll see this pattern a lot.

Classes as a namespace

Over time, developers began separating their code a bit more, grouping like-functions into separate files, and exploring the use of a class-based system for further namespacing. I think this happened for at least two reasons:

  1. Classes can have the same names as functions, but you can store a lot more information in a class, which means you can make big collections of functions.
  2. WordPress developers, after writing functional code for years, were eager to try their hand at object-oriented programming.

Re-hashing the example from the previous section, we might have something like this:

<?php
class JMW_Custom_Post_Types {
    public static function register_product() {
        register_post_type( 'product', array( /* My options... */ ) );
    }
}
add_action( 'init', array( 'JMW_Custom_Post_Types', 'register_product' );

As with the ! function_exists check in the previous example, we might also wrap the class declaration in a ! class_exists check here, too, just to make sure that our JMW_ prefix is sufficient.

I want to reiterate here: this is fine. The thing about computer programming, particularly in PHP, is that there are a lot of ways to do things, and whatever gets your code shipped and out to the world is more important than which particular approaches you used.

That said, I want to tell you about a third approach, because there is currently not a single* example of it in all of WordPress core.

The namespace keyword

The namespace keyword has been a part of PHP since version 5.3. It avoids the prefixing and class-based approaches ot avoiding symbol collisions in the language. Here’s another example of the function-based approach from earlier, but using the keyword instead:

<?php
namespace JMW;

function register_post_type() {
    \register_post_type( 'product', array( /* My options... */ ) );
}
add_action( 'init', 'JMW\register_post_type' );

Notice anything weird? Well, in this example, I’m prefixing the call to WordPress’s register_post_type function with a backward slash. That’s because I need to tell PHP that I want to use a method from the global namespace, which is to say: I want to use the register_post_type method that didn’t declare a namespace, because WordPress does not currently have any namespace conventions in core.

That said, the code in the above example won’t fail because PHP allows us to have two functions with the same name, so long as we identify distinct namespaces for each of them, just like how two people named Travis can work at the same company (sometimes, you might just give them nicknames, even if they have different last names. Nicknames are another form of namespacing.)!

How is this better?

It may not be evident at first why this makes a difference. Adding a namespace to the top of your file might be more work, but what it really does is frees you up from the way things have always been done. If, for example, you’re a plugin developer and you really like using short, concise function names, you can use them without fear of symbol collision from WordPress core or other plugins. Heck, you can use them without fear of symbol collision from your own code!

Let me demonstrate what I mean with a few more examples:

<?php
namespace JMW\Product_Post_Type;

function register() {
        register_post_type( 'product', array( /* My options... */ ) );
}
add_action( 'init', 'JMW\Product_Post_Type\register' );
<?php
namespace JMW\Faculty_Post_Type;

function register() {
    register_post_type( 'faculty', array( /* My options... * ) );
}
add_action( 'init', 'JMW\Faculty_Post_Type\register' );

The above examples might be two separate functions files name product-post-type.php and faculty-post-type.php, respectively. Although they both contain a register method, you could include them both in your functions.php file without worrying about a symbol collision, because they’re each namespaced differently. This gives developers a lot of flexibility to use short function names without worry that there will be a symbol collision, even within their own codebase.

Summary

These examples only really scratch the surface. In the broader PHP ecosystem, namespaces are frequently used not just to prevent symbol collision, but also to dictate class file organization, such as in the PSR-4 standard recommended by the PHP Framework Interoperability Group. In their configuration, every package (e.g., a WordPress plugin) has both a vendor and a package namespace at the top-level:

<?php
namespace JMichaelWard\MyGreatPlugin;

In PSR-4, the above example would be considered the root namespace, which means any files with that namespace could be located in the root of a directory indicated by the developer. Namespaces with extra levels would map to a sub-directory. Example:

<?php
namespace JMichaelWard\MyGreatPlugin\PostType;

The PostType sub-namespace would suggest that I have a directory named PostType where all of my classes are kept. This approach helps simply class autoloading, which is perhaps another post altogether.

Needless to say, namespacing your function and class files in WordPress, even if core isn’t yet doing it, can provide some great benefits. It allows you to confidently use shorter method names (particularly when paired with the <vendor>/<packagename> approach I just described). It allows you to group code into directories when paired with class autoloading. And, least importantly, but important nonetheless: it allows you to use a language feature that has been available in PHP for nearly 12 years.

I hope this post inspires you to experiment with PHP namespacing, and sets you on your journey for further exploration of the server-side language of WordPress.

Footnotes

* WordPress core contains the PHPMailer library, which in fact, uses the native PHP namespace keyword.

How a Conversation About Filterable Autoloader Paths Pleasantly Derailed My Sunday Afternoon

My good friend Gary Kovar posed a really interesting question on Twitter yesterday:

This led to some conversation both on Twitter and separately on Slack, and I began to realize that this dovetails nicely into that series about re-envisioning PHP development for WordPress I’ve been alluding to for months. Consider this post the first in that series.

Dependency Management in WordPress

Here is a real-life problem I encountered on a client project while I was working in agency: the client was using one plugin to save their media uploads to Amazon S3, and they wanted another custom plugin developed that, when creating a new site in WordPress multisite, a hook would be triggered that creates a new bucket on S3 for that site’s media.

The client had one of the most sophisticated build, testing, and deployment setups for WordPress I have encountered yet in my career. They were using Composer for installing their WordPress plugins, and so naturally, when I began building this custom plugin, I created a composer.json file and required the latest version of the Amazon SDK as a dependency, then got to work on building out the functionality to automatically create the buckets.

Everything was going great. That was, at least, until I got to the point where I’d actually attempted to install my plugin alongside all of the other plugins in their multisite installation. It turned out that the plugin that was uploading media to Amazon was using an older version of the SDK, whereas I had coded my plugin against a new version. When their build processes attempted to pull my plugin into the site alongside the other one, everything appeared to work correctly because the other plugin was shipping the Amazon library alongside its code, instead of downloading it using Composer.

If you’re not familiar with Composer, typically what happens when two libraries (or in this case, WordPress plugins) require incompatible versions of the same library, Composer will exit the install process and inform you of the incompatibility. In this case, because the other plugin wasn’t using Composer, I wasn’t aware of the incompatibility.

There’s an additional wrinkle here, too, because in WordPress, the core application does not have a process to check for and resolve all of the dependencies within a plugin or theme. Thus, if one plugin is installing a library via Composer, and another one ships it with the plugin, it’s possible that whichever plugin loads first will load the dependency. In a well-coded plugin, developers guard against loading a library again if it’s already been loaded to avoid throwing errors in PHP, and well written libraries (such as Amazon’s) can avoid throwing errors if there’s good compatibility in their APIs across versions.

This was the case for this particular project. Ultimately, I came to discover that my code wasn’t working correctly because the other plugin’s library was loading first. We wound up forking that library, checking out its beta version (which used the newer version of the SDK and was nearly released), and went on our merry way.

Why is this a WordPress problem?

The short answer is that it’s complicated. Nearly 40% of the top 10 million websites on the internet today are powered by WordPress. Not every WordPress site is maintained by a developer – in many cases, they’re maintained by non-technical people for whom WordPress was an intuitive solution to help them get their vision online. Those folks don’t care about Composer or PHP dependency resolution or namespace collisions. They (rightfully) just want things to work.

For the rest of us, this means we try to come up with innovative solutions to be able to use the tools that make our jobs easier. Because WordPress core doesn’t have a mechanism for resolving a plugin and theme’s dependencies before it loads them, as plugin developers who work in sophisticated ecosystems like that of my former client, we have to rely on alternative solutions like Mozart or Imposter to package our vendor directory with our project and prefix its class namespaces with our own to avoid naming collisions in PHP.

There has to be a better way, but it’s clear that whatever the solution might be, it’s neither easy nor obvious, or as a community, we may have resolved it by now.

Let’s Talk About Extensibility

So, back to Gary’s question and the conversation that followed, which still pertains to workarounds for the dependency management issues in WordPress, but also includes some nuances around class inheritance, interfaces, and incorporating developer guardrails into open-source projects that I find particularly compelling.

In our conversation, we were discussing class property and method visibility in PHP, and Gary shared with me this filter in the MySQL class of SquareOne, an open-source framework developed by the fine folks at Modern Tribe. Though not about autoloading, I’m hoping the correlation here is clear. Let’s look at this snippet (condensed from the link above):

class MySQL implements Backend {
	const DB_TABLE = 'queue';

	private $table_name;

	public function __construct() {
		global $wpdb;

		$table_name = $wpdb->base_prefix . self::DB_TABLE;

		/**
		 * Filter the table name used for queues on this backend.
		 *
		 * @param string $table_name Table name with $wpdb->prefix added
		 */
		$table_name = apply_filters( 'tribe/queues/mysql/table_name', $table_name );

		$this->table_name = $table_name;
	}

There are two important bits in this example:

  1. The private keyword on the $table_name variable
  2. The call to apply_filters, which provides developers with the ability to override the default value of $table_name

Historically, actions and filters in WordPress have been offered as an easy way to override the default behavior of the code that plugin authors write. Like the end users of WordPress itself, developers who work with the platform have a broad range of experience – especially when it comes to object-oriented programming – and these hooks offer an arguably simpler way of overriding the original programmer’s logic.

A neat thing about PHP, however, is that it also accommodates for changes. Consider this alternative to the snippet above:

class MySQL implements Backend {
    const DB_TABLE = 'queue';

    private $table_name;

    public function __construct() {
        global $wpdb;

        $this->table_name = $wpdb->prefix . self::DB_TABLE;
}

But Jeremy… you just took out the filter.

You got me. :)

In this case, MySQL is instantiated somewhere, and it implements an interface, which means it has a whole set of methods that can be called on it. Instead of filtering just the table name, Modern Tribe could consider allowing for modification of the entire class by filtering the places where it is instantiated, and requiring an interface in that callback.

Here’s a contrived example. Let’s say I’m a developer who wants to extend the MySQL class of SquareOne in my own plugin. Let’s assume, for now, that I only want to override the value of the table name. I could create a class that looks like this:

use Tribe\Libs\Queues_Mysql\Backends\MySQL;

class MyPersonalMySQL extends MySQL {
    protected $table_name;

    public function __construct() {
        global $wpdb;

        parent::__construct();

        $this->table_name = $wpdb->prefix . 'my_personal_queue';
    }
}

Notably, PHP allows you to weaken the access level of class properties and methods for extending classes. Thus, it’s okay here to change the visibility of $table_name from private in the parent class to protected here. Additionally, because I only want to change the table name here, my class can inherit all of the other default behavior from the MySQL class. That said, this does complicate both my approach and Modern Tribe’s. In their case, if they want me to continue to be able to override the class, they have to add a filter to wherever that object is being instantiated. It’d look something like:

$mysql = apply_filters( 'tribe/queues/mysql', new MySQL() );

And I, of course, would need to add the filter myself, AND extend the class, when I could have just filtered the table name in the first place:

add_filter( 'tribe/queues/mysql', new MyPersonalMySQL() );

Versus:

global $wpdb;

add_filter( 'tribe/queues/mysql/table_name', $wpdb->prefix . 'my_personal_queue' );

If extensibility is the goal, then there’s an argument to be made that we as open-source developers could be creating more filters in order to accommodate both procedural and object-oriented developers alike. However, in my mind, the argument for making codebases easier to understand and to raise the bar for WordPress developers alike lies in interfaces.

A Case for Interfaces

In his tweet, Gary asked if there were any open-source examples where the path to a Composer class autoloader is filterable. Using the previous examples as a guide, let’s examine what plugin instantiation might look like from the perspective of a WordPress developer who wants to modify plugins with their own autoloaders. Consider the following plugin:

/**
 * Plugin Name: Incredible Plugin
 * Description: Does something incredible!
 */

namespace JMichaelWard\IncrediblePlugin;

require_once __DIR__ . '/src/IncrediblePlugin.php';

function init( Autoloadable $plugin ) {
    $plugin->init();
    
    return $plugin;
}

add_action( 'plugins_loaded', function() {
    $plugin = apply_filters( 'init_incredible_plugin', new IncrediblePlugin( __FILE__ ) );
    init( $plugin );
} );

The plugin itself might look like this:

namespace JMichaelWard\IncrediblePlugin;

class IncrediblePlugin implements Autoloadable {
    private $plugin_file;

    public function __construct( $plugin_file ) {
        $this->plugin_file = $plugin_file; 
    }

    public function init() {
        $this->autoload();
    }

    public function autoload() {
        require_once plugin_dir_path( $this->plugin_file ) . '/vendor/autoload.php';
    }
}

The beauty here is that this code accomplishes a couple of things.

First, the init method in the main plugin file uses a type-hint against the Autoloadable interface. In this scenario, this means that another plugin author who needs to modify this plugin’s primary class in some fashion could do so simply by extending the IncrediblePlugin class (since it already implements Autoloadable), or that – if somehow needed – they could pass in an entirely new plugin object that meets the requirements of the behaviors for the rest of the plugin, so long as it also extends Autoloadable. If Gary simply didn’t want the autoloader to run, he could override the method and simply have his new class do nothing.

Second, what this achieves – and this is notable because no concept like it exists in WordPress core today – is that because the init method in the main plugin file uses a type-hint, if another developer extended it and forgot to implement that interface, PHP will throw a fatal error and warn that developer that they did not meet the requirements to initialize the plugin.

Believe it or not, errors are good. Great, even. Or, I should clarify, that’s the case when they are part of the development process. Errors provide context for what you haven’t done correctly, and through the process of creating interfaces for your classes, you are establishing a contract for what is required to alter that application, whether you’re the one doing the alteration, or if you’ve created a developer API intended for others to extend.

Conclusion

In this post, I covered the following topics:

  • Challenges associated with dependency management in WordPress
  • Proposed approaches for adopting broader filter usage
  • Weakening restrictive property and method visibility during class extension
  • A case for using PHP interfaces

I expect that a few of these topics will surface again in future editions of this series. For instance, the benefits of interface usage in software development are a significant part of the first 5 principles of S.O.L.I.D. Type-hints, too, help ensure that the values we pass into functions throughout our codebases are of the type we expect, and using them can really help us spot errors during the course of developing new features.

I’ll close this inaugural post in the Rethinking PHP Development in WordPress series by thanking Gary once again for the discussion that helped get my wheels turning. As I seek one or more mentors to increase my own knowledge in 2021, I’m reminded that our friends, colleagues, and peers are often the best mentors of all.

Seeking Mentorship

I’ll keep this short and sweet: one of my professional goals for 2021 is to establish formalized mentorships with one or more of my peers in the tech community.

Through the course of my 8-year development career, I have aimed to increase my knowledge and improve my technical skills in a variety of ways. Like others, I’ve read numerous books to learn about new languages, design patterns, tools, and approaches to engineering. I’ve attended conferences and local meetups to build relationships with others in my field, and of course, to attend presentations and discover what additional tools I can add to my skill set.

In July of last year, I joined the product team at Rocketgenius. There, I work on the add-ons crew where I am responsible for developing and maintaining our collection of WordPress plugins that further extend the functionality of the Gravity Forms plugin.

Moving from the agency space into product has long been one of my career goals, because I think it provides me with the opportunity to apply a lot of the “ideal engineering” scenarios that seem prevalent in all that book-learning: test-driven development, continuous delivery and deployment, domain-driven design, agile development, etc. I’ve had exposure to certain aspects of these concepts in my agency work thanks to working with large corporations that adopt them, but because those projects are so fluid, I’ve typically used what had already been established by our clients instead of working to build something of my own.

Needless to say, I recognize that there are shortcomings in my current skill set that I can work to improve upon in 2021, and I think it would be helpful for me to collaborate with one or more folks over the course of the year. In my mind, this collaboration would entail semi-regular (monthly, perhaps?) calls to discuss challenges working in the technical field, and to share resources and ideas around how to grow one’s professional skill set.

Some initial topics that come to mind:

  • Modernizing legacy codebases (in particular, those which can be extended by end users)
  • Containerization
  • Developing and maintaining custom build processes
  • Test-driven development
  • Domain-driven design
  • Engaging with open-source communities
  • Avoiding burnout

If you have expertise in any of these areas and would be interested forming a mentor/mentee relationship, and especially (but not necessarily limited to) if we already know one another, then please get in touch. I think there’s likely a lot we can learn from one another.