Ondřej Mirtes

English version of the article is available on Medium.

Every development team should have a solid supporting infrastructure that helps ensure and enforce consistent output from all of its members. A coding standard is one of the many things that hold a project together.

You shouldn’t be able to tell who wrote a piece of source code. The whole team should follow uniform conventions. Some of them (the way code is formatted) can be checked automatically, others (e.g. adherence to the chosen architecture) need to be addressed during code review.

At Slevomat we’ve had a very strict standard in place for a year and a half now, which Jenkins checks in every pull request with the help of PHP_CodeSniffer. Six months ago, on top of the foundation provided by the Consistence Coding Standard, we added a number of advanced sniffs that we’d provisionally committed to our private repository. Some of them also support automatic fixes, which makes integrating them into a project easier.

I’ve waited a very long time for today. We’re finally releasing these sniffs as open source for public use. Let me highlight the ones I consider the most interesting and useful:

Unused private properties and methods

SlevomatCodingStandard.Classes.UnusedPrivateElements

PHP_CodeSniffer is unsuitable for static analysis of source code, because it can only analyze a single file at a time and has no access to reflection. But some specific checks can be performed with it. This sniff detects unused and write-only private properties and unused methods that can be safely deleted. Thanks to this sniff we regularly get rid of dead code during refactoring, as well as needlessly injected, unused dependencies in the constructor.

Trailing comma after the last array element

SlevomatCodingStandard.Arrays.TrailingArrayComma

A comma after the last element in a multi-line array makes adding further elements easier and makes version-control diffs clearer.

Disallowing Yoda conditions

SlevomatCodingStandard.ControlStructures.YodaComparison

If you prefer the classic ordering when writing conditions instead of the Yoda style, this sniff can find and even automatically fix the offenses.

Alphabetically sorted uses

SlevomatCodingStandard.Namespaces.AlphabeticallySortedUses

A good coding standard should enforce unifying the form of all code that can be objectively unified. This sniff eliminates arguments about how to order the imports from other namespaces at the top of each file.

Unused uses

SlevomatCodingStandard.Namespaces.UnusedUses

More dead-code detection. Why tolerate superfluous uses when this sniff can find them and even delete them? It can handle Doctrine annotations too.

Start using them today!

The sniffs mentioned here aren’t the only ones we released today. You’ll find a complete list of them and an exhaustive guide to using the whole standard — or even just a few individual sniffs — together with the source code on GitHub. We consider the code so well tested and stable that we didn’t hesitate to release version 1.0.0.

Take a look at the following code:

public function processRow(Row $row)
{
	$this->databaseConnection->begin();
	try {
		// some initial checks
		// $isProcessed = ...

		if ($isProcessed) {
			return;
		}

		// lots of work processing the row

		$this->databaseConnection->commit();
	} catch (\Exception $e) {
		$this->databaseConnection->rollback();
		throw $e;
	}
}

Did you spot the bug? When $isProcessed === true, the method is exited without committing or rolling back the transaction, which leaves it open in the database until the connection is closed. And it may needlessly hold locks for too long — locks that other threads of the application might be interested in.

If you don’t mind the locked rows, you still won’t like the next consequence of this bug:

$this->databaseConnection->begin();
try {
	// the called method evaluates the passed row as $isProcessed
	$this->processRow($fooRow);
	$this->databaseConnection->commit();
} catch (\Exception $e) {
	$this->databaseConnection->rollback();
	throw $e;
}

Because in this case you’re committing or rolling back a different transaction than you’d expect!

With a better transactional API, the situation can be detected:

$databaseTransaction = $this->databaseConnection->begin();
try {
	// the called method evaluates the passed row as $isProcessed
	$this->processRow($fooRow);
	$databaseTransaction->commit();
} catch (\Exception $e) {
	$databaseTransaction->rollback();
	throw $e;
}

In this case the database transaction is represented by a separate object, so it’s always unambiguous which specific transaction we’re working with.

But how do we detect that an unresolved transaction is left dangling in the application? A destructor will help us:

class DatabaseTransaction
{

	//...

	public function __destruct()
	{
		if (!$this->resolved) {
			// a destructor cannot throw exceptions
			trigger_error('Unresolved transaction!', E_USER_NOTICE);
		}
	}

	//...

}

But when does PHP call an object’s destructor?

The destructor method will be called as soon as there are no other references to a particular object, or in any order during the shutdown sequence.

Depending on how DatabaseConnection is implemented, the destructor is called either as soon as PHP exits the method in which $databaseTransaction was assigned, or perhaps not — for example, if we store all transactions in an internal array for later reference.

So if the transaction’s destructor is called right after the method is exited, then in the logged error we get a stack trace in which we can easily find the place where the transaction is handled incorrectly, and we’ll be able to fix it.

If the destructor is called only during the shutdown sequence, then the error’s stack trace tells us nothing about where the unhandled transaction originates. If you have a large, multi-layered application with many transactions, such a place is very hard to find.

But I came up with a working solution that makes debugging the transaction easier in this situation. In PHP you can instantiate an exception without throwing it, and an exception created this way carries within it the stack trace from the place where it originated!

class DatabaseTransaction
{

	/** @var UnresolvedTrasactionException */
	private $originException;

	public function __construct(DatabaseConnection $databaseConnection/*, ...*/)
	{
		// ...
		$this->originException = new UnresolvedTrasactionException();
	}

	public function __destruct()
	{
		if (!$this->resolved) {
			\Tracy\Debugger::log($this->originException);

			// a destructor cannot throw exceptions
			trigger_error('Unresolved transaction!', E_USER_NOTICE);
		}
	}

	//...

}

So for every transaction that’s started I create an exception, which I log in case of an unresolved transaction, and thus obtain the place where the faulty transaction originated. In the destructor I still raise a notice, so that the programmer notices this error during development too, when they don’t normally watch the log folder.

In terms of code cleanliness and architecture this is of course a rather unusual, even insane solution — one I’d never let into the regular business logic of an application during a code review, where a developer has to make do with the usual means of expression and design patterns from OOP. But in this case it’s an infrastructural helper that occurs exactly once in the entire application and doesn’t affect the architecture of the rest of the application in any way. To make debugging easier — and thus save the developer’s time — I’m willing to cross a line I’d normally never even approach.

An absolutely epic article of extreme length (38 thousand words) with plenty of animated illustrations that describes everything you need to know about software development. From an understandable explanation of how a processor works and everything that has to happen for a character pressed on the keyboard to show up on the screen, through programming languages, libraries, algorithms, debugging, data structures, databases and versioning, all the way to developer conferences, flame wars and management. If you’re considering a career in software development or already have one, this is truly a must-read for you.

Martin Fowler responds to the current craze for microservice architecture by arguing that starting the development of a new application from microservices is dangerous, because you don’t know up front how to split the project into them, and you’re more likely to hurt yourself with poorly applied architecture.

Any refactoring of functionality between services is much harder than it is in a monolith. But even experienced architects working in familiar domains have great difficulty getting boundaries right at the beginning. By building a monolith first, you can figure out what the right boundaries are, before a microservices design brushes a layer of treacle over them.

Microservices are indeed more advantageous in the long run, because development of the project can be split and scaled across multiple independent teams, but at the same time they carry inevitable overhead in designing and implementing the API, reacting to failing requests, the inability to pull all the data from a single database at once, and so on. And if you’re developing something new, you don’t know whether the project will have enough success and growth to make microservices and a slower time to market worth it.

When you begin a new application, how sure are you that it will be useful to your users? It may be hard to scale a poorly designed but successful software system, but that’s still a better place to be than its inverse. As we’re now recognizing, often the best way to find out if a software idea is useful is to build a simplistic version of it and see how well it works out. During this first phase you need to prioritize speed (and thus cycle time for feedback), so the premium of microservices is a drag you should do without.

David Grudl called on developers on Twitter to compare developing a web application in Nette versus a JavaScript solution. No matter which of the competing technologies won on speed, I hold the view that it says nothing about its suitability for the long-term development and maintenance of serious applications.

These comparisons have a lot in common with synthetic benchmarks. What they test doesn’t correspond to what an application does in normal operation, and at the same time the raw performance of a framework isn’t the main criterion a developer should care about when choosing one.

When developing, what matters isn’t how quickly a developer can hammer out the first version of an application on a greenfield project according to a fixed specification, but a number of other criteria:

  • The application’s extensibility and maintainability, how easy it is to make changes in the source code
  • Testability and test coverage
  • The state of the source code and how easily it can be handed over to another developer or team
  • The friendliness of the user interface
  • The availability and cost of developers familiar with the technology
  • Documentation, community, and the technology’s future

Three years ago I myself took part in the Battle of the Frameworks. The task was, much like in David’s challenge, to develop an e-shop with an admin interface in a single day. Each of the competitors approached the task differently, and the resulting ranking didn’t necessarily correspond to how the given application would fare over the long term. For example, the winner, Jakub Vrána, used his Adminer Editor to build the admin interface, which saved him a lot of time in the competition, but an admin interface generated from the definition of tables in a relational database provides no room for customizing behavior and therefore delivers lower user comfort.

The experience of the individual participants also naturally factors into the comparison – both with programming in general and with the specific competition technology.

So how should you go about choosing a technology, when a one-day hackathon won’t help us pick it? When we started developing a new shopping cart at Slevomat at the end of last August, we knew we wanted the whole thing to work as a single-page application with all server communication happening via AJAX. We needed a technology that would project the state of the data on the server into the template on the client side. As suitable candidates we picked React, Knockout, and the templating engine Handlebars. I spent three days comparing these technologies by implementing the first step of the cart in each one, and then comparing which technology was closest to how development happens at the company, estimating what could become a problem in the future and how easily this or that would help us solve it. I chose Knockout, and two months later we successfully launched the cart without any major problems on the client or the server.

So did I choose correctly? The catch is that this can’t be determined. How would development have gone if I’d chosen React? Maybe the cart would have been finished two weeks earlier, but after launch we’d have run into a nasty, hard-to-debug bug. Maybe we wouldn’t have made the deadline. Maybe it would have been more performant at redrawing the DOM, but the front-end coder wouldn’t have been able to edit its templates. If we really cared whether we’d chosen well, we could try developing in both technologies in parallel for the entire two months, which on one hand would cost more money, and on the other we’d again run into the differing levels of developer experience, so the comparison once again wouldn’t be objective.

If, after finishing the cart in Knockout, I were curious how React would fare and set out to implement it in React myself, I again wouldn’t be able to compare the technologies objectively, because while developing the first version I’d gathered experience that I’d apply in React, allowing me to avoid some dead ends. A fair comparison therefore can’t be obtained not only from several different developers, but not even from a single one.

When choosing a technology, take into account what your expectations of it are and whether it can meet them, the skills and experience of the people who’ll be working with it, and don’t agonize too much over musings about how the project might have turned out if you’d decided differently at the start. [1]


  1. And definitely don’t look at performance benchmarks, popularity polls, and what your favorite personality uses. ↩︎

‹ Year 2016 Year 2014 ›