When isTrue is true but not quite

From time to time I see the following code pattern (note line 4):

$isTransportable = !$item->material->isHazardous()
    && !$item->container->isPressurized();

if ($isTransportable == true) {
    $transporter->transport($item);
}

Sometimes, this may be due to changes in legacy code applied without looking at the bigger picture. However, other times this seemed more intentional than accidental, in which case I consider it a bad pattern for the following reasons:

  1. Is true really true? Maybe wrap it in some brackets and check that the result is true? trollface
  2. It is not idiomatic – in practice, you’d say “if transportable, do something” not “if transportable is true, do something”
  3. It is harder to read: variable, operator and value – what one might call as visual debt
  4. It is unnecessarily verbose – the short version is just as readable
  5. It is technically ambiguous – this is more of a problem with PHP, however that condition actually checks against truth-y values (true, numbers except 0, non-empty strings, non-empty arrays, objects)

So I should just switch to identical operator (===), right?

Not necessarily. In cases where you specifically expect a boolean state, there’s no reason to be pedantic about the real value. Remember, garbage in – garbage out: If you get a 1 instead of true, the behaviour might not be as desired either way. Also, if you really have to have some checks, put them at the very top and throw an InvalidArgumentException instead (or use type declaration in PHP 7).

But I don’t like boolean type coercion

Deal with it 😎. It’s not just PHP that allows type coercion and nearly every language that does also agrees more or less on the behaviour (of course, as usual PHP has it’s own special behaviour). Anyway, you can always ensure the type upfront instead of verifying values inside conditions later.

What if I want to check for a false? An exclamation mark is too short

And? The exclamation mark as a logic negator is a very common operator in many languages. Also, if you really want to be more verbose and idiomatic, you can also use not: if (not $isTransportable) {.

Bottom line

Thanks to some standards and accepted practices, one can write concise code without worrying about making it unreadable. Verbosity might help a novice developer, but in the long run will make large swats of code larger and harder to read by any audience.

PHP Session Lock

As some of you may know, I’m currently working on an ambitious project to audit WordPress events.

One of the features I eventually added, is the ability for the log viewer to update results in (almost) realtime as soon as they’re sent to the server.

I achieved this using the Comet (also known as “long polling”) method. In simple terms, it works like this:

  • the client asks the server if their are any updates
  • the server check for updates at intervals
  • if changes are found, they’re sent to the client, otherwise the request is terminated after some time
  • the client processes any received updates and performs a new request (restart this process)

To get this working, there are a few prerequisites, such as, the request should take as little memory and processing power as possible. Or, in other words, the impact on the server should be minimal. Here’s a small snippet on how it’s done:

// get the number of events the client knows about
$old = (int)$_REQUEST['logcount'];

// the number of tries (multiply this by the duration for usleep and make
// sure it is less than 30s since servers, firewalls, routers and some web
// browsers are known to drop HTTP connections taking more than 30s)
$max = 40; // 40 * 500msec = 20sec

// check for new events in db by monitoring event count
do {
	// count the number of events in db - make sure this impacts the DB
	// and server as little as possible.
	$new = count_events_in_db();

	// give the server some time to breath (500msec)
	usleep(500000);

} while (($old == $new) && (--$max > 0));

// print out the new event count or 'false' if none
die($old == $new ? 'false' : $new);

Anyway, we’ve got it working after a few test runs and off into production. It worked well for a while.

And then, one fine morning, we started getting all sort of weird issues. Sluggish site performance, client requests timing out etc…

Worst of all, system metrics looked fine: cpu usage, memory, network, disk… and no error log entries. In short, everything looked ok.

Eventually, we found an indication of the culprit. Well, when I say “we” I really mean Robert Abela. 🙂

It seemed that a particular plugin seemed to be causing the issue, somehow. I installed it locally and ran it through the XDebug profiler.

After a couple of runs, a stack trace was caught when a time out occurred, finally shedding some light on the source of the problem: it was simply a call to session_start();

 

Admittedly, my experience with session in PHP is a bit limited. I know the basics, but that’s it. So, I did a quick search on the internal effects of this function. Guess what? PHP makes use of a session file which it locks to keep the data integrity for sessions intact. It’s actually a very good thing to have, but alas, in my case, I wasn’t even using sessions. The problem was the long-polling request, sometimes, locked this file and waited for updates for up to 30s. This means any subsequent request that enabled sessions had to wait for the comet request to finish.

The solution is relatively simple: at the start of the comet request, one simply had to call session_write_close(); it basically tells PHP to stop writing session data and consequently release the lock.

Props to Konr Ness and his excellent article on the matter.

PHP Post-Mortem

PHP is very configurable, but sometimes default settings or those imposed by the framework in use tend to make simple debugging very difficult.

Don’t get me wrong, whenever possible a proper PHP Debugger (extension) is the way to go, but sometimes, when all else fails and you’re facing a blank white page, the snippet below can get you out of the situation in no time.

So, how does it work? While a long time have passed since I first wrote it, little changed in the way it works.

As you might know, in PHP you can register a shutdown handler – a function to be called when PHP is closing down. What you might not know is that this function is called no matter what; even when fatal errors come up. The only exception is if an existing shutdown handler decides to kill the remaining handlers from executing (which is usually rare).

This functionality makes it a good candidate for retrieving interesting information on the current page run, such as:

  • where the initial output started from (useful to find files that cause PHP to start the output)
  • last error message (useful to see fatal errors)
  • the loaded files (useful to find which file caused PHP to stop)

All this is conveniently rendered inside an HTML comment to (hopefully) prevent any end-user from seeing anything out of the ordinary. Of course, use this script with care, it may easily tell too much about your server to the world!