When was the last time you’ve sat down and thought about your career? Do you like what you do? Are you getting what you want out of it? Does it challenge you in ways you want to be challenged? Are you thriving, or simply surviving?

I love writing code. If you’ve read this blog for any length of time, you know that I’ve worked primarily in the WordPress space for the last 8 1/2 years, and that I’ve simultaneously kept my feet firmly planted in the broader developer communities. I’ve done this because I feel that it’s important to get a full range of perspectives to inform my craft, and to determine whether there’s something someone is saying from outside of WordPress that could help improve the way we do things within it. I love writing code, and I’m also passionate about software architecture and finding ways to reduce the friction involved in maintaining code. This is a big reason why I’ve focused my attention over the years on concepts like modular development, the five principles of S.O.L.I.D., writing clean code, incoporating testing into your workflow, etc.

I write about these topics here not because I think of myself as an expert in any of them, but because I find them interesting and I think that by discussing the way in which we write code, we can start to consider whether how we write it today is serving us both now and in the future. How difficult is it to change your existing codebase because of some design decision you made in the past, because you didn’t write tests to support it, or because you didn’t document it, or perhaps all three and you’re maintaining a codebase that wasn’t even written by you?

There’s a lot of friction in writing software. In my experience, that friction is lessened over time through coding practices such as code review and pair programming, particularly on small teams, because those are the times in which we can discuss our craft and the current state of our projects, share ideas, and come to consensus about the way in which we want to do things going forward. No two people have the same pool of knowledge, and only by encouraging these practices do teams, over time, grow together to understand the existing state of their codebase and agree upon a vision in which they want it to evolve.

There’s also friction in change. Just think about how passionately people have been opposed to the WordPress block editor! It’s come a long way since it was first released a few years ago, and what I think is most exciting about the project is more willingness to make drastic changes to the way we do things: the chance to try something new and break out of the status quo.

As a fan of PHP, I wish that same mentality transferred to the server-side language. In early 2019, WordPress bumped the minimum required version of PHP from 5.3 to 5.6, and stated as a goal that it was aiming for 7+ as a minimum by the end of that year. Around the same time, there was also some discussion about how we could now, finally, start to incorporate some modern syntax into the core application. Two years later, the same minimum version and much of the ways we write PHP in WordPress remains the same.

Going Forward

All of this is a long way of saying that there is a lot of momentum and focus on JavaScript: not just in WordPress, but everywhere. I need to acknowledge that momentum, accept change myself, and determine what all of this means for me.

At the start of my career, I worked on a broad range of projects, building full sites atop WordPress and Shopify for small businesses and non-profits. I enjoyed this work because it exposed me to a variety of technologies and I was learning something new nearly every day. I collaborated directly with designers and my clients to help meet their visions, and when my projects were finished, I got to launch them for the world to see and use. In fact, just two weekends ago, my partner’s aunt told me that she visited the Glensheen Museum in Duluth, purchasing tickets via the site I built several years ago during my time at a previous agency.

For the last 4 1/2 years, I migrated into plugin development, both at agencies and on the product side. I still learned a lot, and in fact, learned so much from one client in particular that it helped further shape my curiosity and interest in the software development lifecycle and software architecture in general. But, due to the nature of the kinds of projects I worked on, I rarely got to build something that I could celebrate and share with others. I’ve felt further and further isolated from the advancements in WordPress, few of which have come to the language in which I’ve specialized.

I have been only surviving, not thriving.

Last spring, I began studying Python with the goal of landing an engineering role outside of my comfort zone. Interviewing was going really well, but then the pandemic hit, and hiring ceased. This summer, I am continuing my Python exploration, brushing up on Node.js and React, getting better at understanding Docker, digging deeper into CLI scripting, refactoring some existing projects and thinking up new ones. I attended DevOpsDays Minneapolis (online) and was completely in over my head with terminology and concepts I have had virtually no exposure to during my time in WordPress, and I loved every minute of it.

I am motivated to get back to the time when working on projects was delightful; when I could launch something new and show it off to my friends and family. I want to write modern code, to collaborate with my peers, discuss pros and cons of various architectural approaches, and create beautiful, well-tested, and stable applications. And I hope to do so at a company, like so many companies that I have already worked for, who value a positive and supportive engineering culture.

I know I can get there. Perhaps it could be with you? If you or your company is hiring and you think I might be a fit, then I’d love to chat. Please get in touch.

A friend and I were having a really interesting discussion this week about PHP foreach loops, array_* functions, and the subjectivity around code readability; significantly interesting enough, in my view, that I wanted to document it here and share it with my broader network for consideration and discussion.

Let’s say you have an array of options you’ve assigned to a class property:

$this->options = [
    [
        'text => 'Good',
        'value' => 'the-good-one',
    ],
    [
        'text' => 'Better',
        'value' => 'the-better-one',
    ],
    [
        'text' => 'Best',
        'value' => 'the-best-one',
    ],
];

This array gets used in a system to select a default option for the list. The calling code searches within the array to see whether there is a match, and if there is, it returns the value of that match. So, for instance, if I passed the string the-good-one to my search lookup, I would get the-good-one back.

If someone wanted to get the value using the text key, the same behavior applies: I pass in Good, and once again, I get the-good-one back. If, then, I pass in something that’s not located within the array, I simply get back the value I passed in – the-worst-one or Worst would return the-worst-one or Worst, respectively.

You’re assigned with writing a fuction that performs these steps. How do you solve it?

The Loop Approach

One possible algorithm for finding a matching value in this set is to use a loop. It might look something like this:

public function get_matching_option_by_value_or_text( $value ) {
    foreach ( $this->options as $option ) {
        if ( $option['value'] === $value ) {
            return $value;
        }
    }

    foreach ( $this->options as $option ) {
        if ( $option['text'] === $value ) {
            return $option['value'];
        }
    }

    return $value;
}

Loops are one of the first programming control structures that new developers learn, and they can help make solving these kinds of problems easy, but they are not without their tradeoffs. Though simple, this example does contain some inherent complexity, primarily: 1) moderate nesting of control flow, and 2) clear-ish but not necessarily full clarity of developer intent (reading the code, you have to stop and consider why there is a return statement within the loop).

The complexity here is that the main point of the algorithm is to determine if a given value doesn’t exist in the value index, and find the matching value for it if it does exist in a text index.

Let’s look at an alternative using built-in PHP array methods.

The PHP array_* Approach

The same result using a loop control flow can be achieved with two built-in PHP array methods: array_search and array_column. The array_search method returns the index of the found result or false if the result could not be found, and array_column pulls all of the values out of an array into a flat structure, preserving the keys. Let’s take a look:

public function get_matching_option_by_value_or_text( $value ) {
    $value_index = array_search( $value, array_column( $this->options, 'value' ) );

    if ( false !== $value_index ) {
        return $value;
    }

    $text_index = array_search( $value, array_column( $this->options, 'text' ) );

    if ( false !== $text_index ) {
        return $this->options[ $text_index ]['value'];
    }

    return $value;
}

There is both more and less complexity in this approach: more because there are two methods within the algorithm that fewer developers might be familiar with, and because we’re making some nested inline function calls (e.g., calling array_column as the second parameter to array_search). It also has more variable assignments, once because we want to know if we found the index of the value key, and again because we want to know if we found the index of the text key.

That said, at the same time, there is also less complexity, because the control flow is subjectively easier to follow: I simply need to read top to bottom to understand what’s happening, and if I need context around the specific functions being used, I can read the PHP documentation to understand how those functions work. In plain language, I can see “If there is a value index, return the value that matches it from the options. If there is a text index, return the value that matches it from the options. Otherwise, return the value.” The intent behind this approach, I’d argue, is clearer, because any questions that get raised as a result of the algorithm can be reviewed within the documentation, whereas questions about the first approach might need to get answered by running the code and stepping through it with a debugger, or asking the original developer who wrote it.

Programming “The Right Way”

Spoiler alert: there is no “right” way. The beauty of programming is that there are lots of different ways to tackle problems. To me, the most important thing is that when someone is assigned at some future date to modify code you’ve written in the past, how easy is it for them to understand what you’ve written? In my view, it’s easier to follow control flows that have reduced levels of nesting, and which use native language contructs. However, I’m just one person, with one worldview, and one collection of knowledge. Everyone’s technical level and programming approaches differ, so it’s important not to be dogmatic about a particular approach, and appreciate that broader audiences might understand how things work in a different way.

At the end of the day, the one true measure of your algorithms is how many tests are supporting it, so that you can more confidently and easily change it when the tides shift. :)

Addendum

I edited this post after a conversation with another friend, Sal Ferrarello, as I’d realized the original examples for the loop were incorrect. Both the loop and array_search examples have been revised to be the same: returning the $value if it was found in the 'value' index of the original array, otherwise returning the 'value' index if $value was found in the 'text' index. Finally, the methods both return the $value parameter if it was not returned in either search. Thank you, Sal, for noticing the inconsistencies in the original post.

There are, also, notably some limitations with array_column in PHP, which does in fact make the loop example preferable. The array_column method creates a new array from the values, so if any does not contain that index, the indexes of the new array will not match the one of the $options array. Thus, it would seem, once again, that the simpler approach, even with its additional “grok-factor” (because of the early returns in a loop), might in fact be the superior one.

This blog post is more for my own future recollection than anything else, but I figured it might be useful for some of you, too.

If you’re used to working with multiple stages of development (e.g., “local”, “develop”, “staging”, “production”), it’s useful to be able to easily migrate files and data between them. For the longest time, I used a tool called Wordmove to do this (or, to quote Mitch Hedberg, “I still do, but I used to, too”, just not for my own site).

What I simply love about Wordmove is how easily it makes it to move plugins, themes, mu-plugins, and the WordPress database itself between environments. Unfortunately, since I switched to a Composer-based install and moved my WordPress installation into its own directory, Wordmove is no longer an option. I’ve simply dealt with this for the most part by being annoyed by logging into my server, exporting the database, downloading it, then re-importing it on my local.

Fortunately, there’s an easier way.

If you didn’t know already, WP-CLI has a db export command, and I use it all the time. You might not also know that you can set up WP-CLI with a configuration file where you can define login credentials for different environments.

In my local development instance, I now have a wp-cli.yml file that looks like this:

@production:
    ssh: <my-username>@<my-server>/<my-production-wordpress-server-path>

What this allows me to do is run any WP-CLI commands from my local installation against my production installation. Those commands will look something like this:

wp @production post list
wp @production rewrite flush
wp @production db query
wp @production shell

The most important command for this blog post, however, is wp @production db export – I want to get an export of my production database and be able to pull it down into my local instance.

Normally, when you export a database using the wp db export command, it writes a file to the directory from which you called the command, and you can give it a <file> option so that it has whatever filename you choose when you’re done. However, you can also pass a simple “-” option to the command to tell WP-CLI to write to stdout, which is perfectly useful for importing the database locally.

What does this mean? It means that using a combination of WP-CLI commands, I can export my production database directly into my local database, then rewrite the URLs to match my local setup:

wp @production db export - | wp db import - && wp search-replace '//jmichaelward.com' '//my-local-domain'

Voilà!

Update

Thanks to Alain Schlesser for the following recommendation:

Did you notice that the search-replace command actually has an –export flag, so you can use it as a replacement for the db export command?

— Alain Schlesser (@schlessera) June 6, 2021

This means that it’s possible to shorten the above command by excluding the export command altogether and using the --export flag on the production URL.

wp @production search-replace '//jmichaelward.com' '//my-local-domain' --export | wp db import -

I tested this locally and it works great! --export accepts a file name as an option, but if you exclude it, it writes to STDOUT just like wp db export does, so it’s ultimately a combination of the two statements.

Because wp db is such an explicit command, I hadn’t considered that there might have been an export option as part of the search-replace command. It looks like this issue was introduced for discussion and ultimate inclusion into WP-CLI quite some time ago, where it was ultimately decided to include this flag on the search-replace command for performance reasons (plus, it has fewer flags!).

All that said, the only thing that gives me pause about using the above approach is that, if one were in a hurry and forgot to include the --export flag, theyd’d be left with a production database that had the replaced string everywhere. As with anything, there are ways to mitigate this, such as wrapping this command in an alias so one needn’t type it out each time, but knowing my own wariness about doing any modifications to a production database, I may personally continue to use the export -> import -> replace approach. However, the above demonstrates just one of the many powerful aspects of WP-CLI, and whichever approach you use in your own work, hopefully this adds a wonderful new tool to your toolset.

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.

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.