Under the hood of Brain Monkey v2

Few days ago I released Brain Monkey version 2.

Many of the changes were kind of “maturing” into my head since quite a long, but most of the code was written in a couple of days; my way to vent the anxiety for my first baby to born.

There were quite a few breaking changes, even if most of the API changed just on the surface. But the code under the hood changed radically, I would say.

In just few hours after the release some good people took the time to ask questions and clarifications. I received issues and PRs, and did discussions about some of the changes. That was awesome: gave me the impression that this library (whose version 0.1 was written in few hours out of frustration), is actually helpful to someone that’s not myself.

This post will provide more insights about the changes under the hood (detailed migration guide is available here) and will also provide the rationale behind some of the changes.

Quantity VS quality

Looking at the repository number of v1.4.2, the version on v1 series when I released v2, it is made of:

• 7 classes
• 963 lines of code
• 7.13 average cyclomatic complexity by class

below there’s the maintainability graph generated by PhpMetrics (size of the circle represents the cyclomatic complexity. Color represents the maintainability index, large red circles are hard to maintain):

The same numbers for v2 tell a different story:

• 31 classes
• 1919 lines of code
• 3.25 average cyclomatic complexity by class

And this is the the maintainability graph:

The raise in classes number looks like huge. However, this is a bit misleading. In fact, in v2 I introduced a lot of custom exception classes, whereas in v1 all raised exceptions were core SPL exceptions.

Each custom exception class count as one, but it is pretty much logicless and some of them are just empty (no methods).

Ignoring exceptions, the numbers for v2 change a lot:

• 15 classes
• 1519 lines of code
• 5.38 average cyclomatic complexity by class

And this is the the maintainability graph:

Comparing these last numbers with v1 numbers, it is pretty clear that I increased by 157% the lines of code and reduced by 25% average cyclomatic complexity by class.

All of this was done while introducing new features which brought a non-trivial amount of complexity: without the new features, just doing refactoring of what was in v1, the average cyclomatic complexity by class had probably been halved or so.

Besides of numbers, believe me when I say that that huge red circle in the graph of v1 represents a class that was a something impossible to maintain.

By contrast, the biggest yellow circle in the graph for v2 above, which seems to be the hardest to maintain class of the library, represents the class Hook\HookStorage: not the simplest in-memory storage implementation one can imagine, but surely not a maintainability hell.

To summarize, in v1 there was one single class that took care of pretty much all the logic for testing WordPress hooks. This logic has been split in five main tasks:

1. a storage for hooks added / executed: as data source for has_action / has_filter
2. a stack for currently executed hooks: as data source for current_filter / doing_action / doing_filter
3. a Mockery expectation factory, used to create Mockery expectations for each function, hook added, and hook executed
4. an “executor” which enforces validation of Mockery expectations when do_action,apply_filters, add_action, and add_filter are called (these functions are declared as alias to the executor object)
5. an Expectation object that acts as a “bridge” to Mockery expectations acting as a “guard” against not allowed Mockery expectation methods.

The first four tasks were done by a single class in v1, in v2 each task is done by a class and this not only reduced complexity and improved maintainability, but enabled the introduction of new features (like the support of doing_action and doing_filter) and the fixing of annoying issues like the break of default behavior of apply_filters when expectations were added for filters.

Other complementary tasks like:

1. naming (functions, hooks) normalization and validation
2. callbacks string representation

which were hidden in the v1 god class, has been split in separate classes, adding correctness and new features without increasing complexity of objects devoted to “main” tasks.

Rationale behind some of the changes

This post is not meant as a release post nor as a migration guide (a detailed one is available here), but more as a “behind the scenes” extra, and I want to include here answers to some of the early questions I received about the v2 changes.

I might update the post if more questions will come.

Verbose closures string representation

A feature Brain Monkey provides already since v1, is to check for hooks added using closures, without holding a reference to those closures.

This is done using a syntax like:

static::assertTrue( 
    has_filter('some_filter', 'function()')
);

See documentation for more information.

In v1 all the closures were represented by the string "function()", in v2 it is required to include the closure parameters (eventually with their type declaration) in the string representation, for example:

static::assertTrue(
    has_filter('some_filter', 'function(array $foo, $bar)')
);

which means that checking for a closure is more verbose in v2.

So, why that?

To answer, I need to go back at Brain Monkey v0.1, the first release that was made of a single class of 100 lines of code, more or less.

At that time, the only way to check if a given filter was added using a closure was:

Filters::expectAdded( 'some_filter' )
  ->atLeast()
  ->once()
  ->with( \Mockery::type('\Closure') )
  ->andRetunUsing(function($arg) { 
    return $arg
  });

This was really too much.

So I got the idea of the "function()" trick in combination with has_filter to reduce this to just:

static::assertTrue(
    has_filter('some_filter', 'function()')
);

However, in Brain Monkey v2 ->atLeast()->once() is the default when adding hooks expectations and the default return behavior of filters (return first received argument) is also there by default.

Which means that in v2 is just possible to do:

Filters\expectAdded( 'some_filter' )
    ->with( \Mockery::type('\Closure') );

that is just few characters more than the has_filter + "function()" way, and has exact same effect.

Considering that to provide the has_filter + "function()" functionality is not free (it comes at a cost of additional complexity, and it is less trivial than it seems) I asked myself if it was worth to keep this feature just to save typing few characters or maybe if it was better to deprecate functionality and remove in next version.

Then I thought that it could be interesting to provide a way to distinguish one closure from another (something that is not possible via Mockery::type): adding signature recognition to closure string representation, even if not perfect, seemed a good approach in compromise between added costs and benefits.

To make the signature recognition possible, but optional had required a fair amount of additional complexity, so I made the signature mandatory, after all who don’t need to distinguish closures can use \Mockery::type('\Closure') that is not big deal and even benefits of IDE auto-completion (unlike "function()").

Functions (instead of static methods) for API

One of the breaking changes with biggest effect on existing code was the change from static methods to functions for main API entrypoints.

For example, what was:

Actions::expectAdded( 'init' );

became

Actions\expectAdded( 'init' );

In v1, Actions and Filters were classes extending Hooks: a big, complex class that actually held all the logic.

In v2 the Hooks logic was migrated to different smaller classes (as already described in the post) and, after migration, the static methods used as API entrypoint were left the only methods of the class.

So I had an Actions class with just two stateless static methods:

• expectAdded()
• expectFired() (later renamed to expectDone)

and a Filters class with just two stateless static methods:

• expectAdded()
• expectApplied()

Looking at those empty classes I remembered what a better and wiser developer than me once said, (stateless) static methods are “pure functions disguised in an awkward syntax and clumsy namespacing scheme”.

I also considered that by using classes I also needed to have two different files for autoload purpose and PSR-4 compliance, so I just thought it has been better to use real functions and so:

• remove two files containing two quite empty classes
• get the chance to have a single api.php files that contains all the API entrypoint functions of the package: that files alone would suffice as advanced API documentation (it will be better better when I’ll get the time to add more doc block to the functions).