On writing clean code

Published in

Cultured Perl

4 min readApr 20, 2017

There’s a little-known secret in the software industry. If you’re in the industry long enough, talk to other people in your field, and maybe head to developer conferences, you’ll hear the secret whispered about from time to time.

Software is awful.

Software projects have deadlines, changing specifications, unclear specifications, changing software developers, unforeseen circumstances, and so on. Many times we’ve heard horror stories of well-known software products which can’t fix certain bugs or can’t develop certain features because they’ve cut corners on their software development.

That’s why the Tau Station development team has been working very hard at following best practices so that when the inevitable bugs surface, we’ll be well-places to deal with them. We follow the thin controller/fat model style of MVC development, we’re at almost 96% statement coverage in our test suite, but one thing I think we’re very proud of is the declarative nature of our “economic exchanges.”

Even for software developers, that last bit may sound odd. The “economic exchanges” are something we’ve touched on before, but they’ve been expanded and the syntax simplified further. Basically, an economic exchange involves transferring goods or services from one party to another, in exchange for desired goods or services. We’ve modeled this in our software.

But first, let’s look at the Perl code which used to be executed if you visited a clone vat and gestated a new clone (truncated slightly for formatting reasons):

sub gestate ( $self, $character ) {
     # must be in the clonevat
     croak( … ) unless $character->area->slug eq 'clonevat';     # must be able to pay
     my $price = $self->price_per_character($character);
     if ( $character->wallet < $price ) {
         $character->add_message( … );
         return;
     }
     my $guard = $self->result_source->schema->txn_scope_guard;     # take the money and gestate a clone
     $character->dec_wallet($price);
     my $clone = $character->find_or_new_related(
         'clones',
         station_area => $character->station_area,
     );
     $clone->$_( $character->$_ )
       for $character->physical_stats;
     my $now = DateTime->now;
     $clone->clone_date($now);
     $clone->gestation_date(
        $now->clone->add(
          seconds => $self->gestation_delay($character)
        )
     );
     $clone->update_or_insert;
     $character->add_message( … );
     $guard->commit;
     return $clone;
 }

That’s a great example of why so much software is bad. There are many behaviors in that which should be rewritten, and there’s the rather glaring issue that this was a method on our clone class. Why the heck was it rummaging about in the character’s wallet? The clone class should only be for gestating and spawning clones. This was not only a gross violation of the single-responsibility principle, but how the heck do we efficiently share those behaviors?

Now that code looks like this:

sub purchase_clone ($self) {
   my $exchange = $self->new_exchange(
     slug            => 'purchase-clone',
     success_message => 'You have purchased a new clone',
     failure_message => 'You could not purchase a new clone',
     Steps(
       Location( $self => is_in_area => 'clonevat'              ),
       Wallet(   $self => pay        => $self->price('cloning') ),
       Clone(    $self => gestate    => $self->station_area     ),
     ),
   );
   return $exchange->attempt;
 }

This is the same behavior as seen in a previous blog post, but it has a subtle difference. Instead of passing a complex data structure to the economic exchange, we have a series of steps, each named after its “topic area” (Location, Wallet, or Clone) and containing what is essentially a sentence in a subject-verb-object (SVO) structure. In the code above, “$self” can be read as “your character”, so the steps become:

Your character is in the clonevat
Your character can pay the price for a clone
Your character gestates the clone in that station area (the clonevat)

When the $exchange->attempt method is called, the purchase-clone slug (a human-readable unique identifier) is used as the key for all the logging. The exchange then iterates over each of the steps in succession, and if any of them fail, all changes are discarded and the failed transaction is logged. If all of the steps succeed, every object which was updated is then stored in the database and we log a successful transaction.

Writing an exchange is easy, as is reading it. Not only is that code much shorter than the method we first showed, but here are a few things we no longer need to write and that the exchange provides us for free:

Individual success or failure messages for the various steps
Logging of the transaction success or failure
Database transactions (ensuring that data only hits the database on a successful exchange)
Fatal error trapping (which might occur if people try to cheat)

By having simple code which is easy to write, easy to read, and handles most of our common cases for us, we can develop our software faster, and if we find a bug in one of the steps, we can fix that step and have many different economic exchanges fixed at the same time.

Originally published at taustation.space on April 20, 2017.

On writing clean code

Written by Tau Station