An Accidental Engineer

A quick dive into Darktable

noreply@blogger.com (Bob McKee) — Sat, 08 Oct 2011 17:37:00 +0000

Full disclosure: I am not a professional photographer or graphic designer. 100% hobbyist.

That said, I do try a fair amount of applications for photo editing and use a fair amount of the tools they offer for image manipulation. Consider my thoughts as coming from the perspective of a hobbyist interested in getting great results (by my standards) with a relatively small investment in learning new software. I'm also use to working with Adobe Lightroom and have a Canon Digital Rebel XTi.

Over the past year I've switched from being a lifetime Windows user to an Ubuntu user. Since the switch I've been desperate for good photo editing/management software. Previously I'd used Lightroom. I love love love Lightroom. I had a few options to continue using Lightroom. I tried going the Windows virtual machine route but I found its responsiveness to be too slow. I felt like the dual-boot route was too much effort. File that complain under First-World Problems. I like my desktop setup and I don't want to reboot my computer just to use one app. I'm lazy.

Finally, I like to not spend money. Free software really appeals me. This lead me to look for Lightroom alternative that would work with Ubuntu and ideally would be free.

The post isn't about my application selection process but I did try Rawstudio and RawTherapee before choosing Darktable. I will say my preference for Darktable was immediate, though there's a few things to know to dive in and get to the "learning by using" part.

Keyboard shortcuts

I like shortcut keys. If you also like shortcut keys and are use to Lightroom you'll find that some shortcuts are the same but a lot aren't. If you want to see what the shortcut keys are and even change then to whatever you like here's how:

I also work on a laptop with a touchpad. You have a lot of zooming control with your scroll wheel, but I don't have one. Without a scroll wheel you need to rely on Alt+1. Note, if you hit it twice it zooms in twice. That is not obvious from just seeing the binding on the shortcuts tab.

Plugins

Each type of adjustment you can do to an image is broken into a plugin. Select a photo and go into Darkroom mode by hitting D. At the bottom right there is a "more plugins" button. Hover over the plugin to find out what the plugin means because the icon does not make it obvious at all. This process annoys me as it takes a fair amount of time just to know what your options are. Clicking on a plugin will turn the icon background grey indicating that the plugin has been added the the right panel. It will be under whatever the plugin's default category is. Clicking the icon twice will turn the icons background red. This means the plugin has also been added to the "favorite" category in the right panel. Click a third time to remove the plugin from the right panel.

Generally I use a few plugins and I use them a lot. My preference was to only add those plugins to the panel and make them all my favorites. This makes it a lot easier to use without having to keep switching between these kinda of poorly defined categories.

Time to play

With my shortcuts ready and plugins out where I can see this its easy to play around and make that transition from Lightroom to Darktable. Darktable doesn't have all the feautres and the easy of use that Lightoom has, but its still damn good and its free. I am extremely excited to see how this application will evolve over the next few years.

PCI DSS compliance and spaghetti code, Part 3

noreply@blogger.com (Bob McKee) — Tue, 09 Aug 2011 13:15:00 +0000

(Be sure to read Part 1 and Part 2 first!)

When writing new code I find its a good idea to start at your integration point and then write that client code, whether just by psuedo-coding or writing some test cases. For my payment gateway code I need code that takes user input, builds a request from that input, sends that request to the payment gateway, and gets a response back so we can act accordingly.

$factory = new RequestFactory();

// get sale builder
$builder = $factory->buildSaleRequest();

// get credit card, so we can set it
$builder->withCreditCard() 
    ->setCardType($_POST['cardType'])
    ->setCardNumber($_POST['cardNumber'])
    ->setExpirationDate($_POST['expirationDate']);

// get address, so we can set it
$builder->withBillingAddress()
    ->setCity($_POST['city'])
    ->setState($_POST['state']);

// send request to payment gateway
$response = $builder->execute();

This seems pretty simple. I have objects to accept my user input which execute the request and get the response. Marvellous. Under the hood its slightly less simple if you aren't particularly familiar with the Factory pattern, Builder pattern, Gateway pattern, Composite pattern, and Visitor pattern.

Today I'll start at the surface with our creational patterns being used here, the Factory and Builder pattern.

Factory pattern

Factories, like all other creational patterns, create new objects. At first they are often understood as a way to make its consumer get different types of object, which is correct, but people fail to see that they are places to return differently configured objects (potentially of the same type) as well. Factories are the place to stowaway configuration information. In Factories the configuration can be centralized, rather than forcing the consumer code to have to repeat configuration each time. For example:

class CarFactory {
    public function create() {
        $car = new Car();
        if($this->config->useElectricCars()) {
            $car->setEngine(new ElectricEngine());
        } else {
            $car->setEngine(new EnvironmentDestroyingBeastEngine());
        }
        return $car;
    }
}

We encapsulate the querying of the "config" object into this Factory method so we don't need to have other parts of the system be aware of this "config" or aware of any configuration of this object at all.

In the case of the payment gateway RequestFactory, we'll be grabbing the payment gateway credentials, which get used in each request, and passing them onto the Request object that gets returned for consumption. This way other code need not worry about where those come from. We've made that decision and we've encapsulated it in a Factory.

Builder pattern

Most people I've seen learning about the Builder pattern don't really get it for a while. This was true for myself as well. We know what it is basically - a class with a bunch of methods you can call to configure an object and then a method to return the newly created and configured object - but don't know when to use it. A quick look at the pattern summary talks about varying complex creation processes. Yea, it can do that, but you have good reason to create those Builders even if you need only one implementation now.

Here are a few indicators to help you know when its time to use a Builder.

Indicator #1: Wiring together collaborators in application code

Are you doing this in your application code? You've written a library to create cars and car parts and everywhere you use this library you're repeating this process...

$carPartFactory = new CarPartFactory();
$carFactory = new CarFactory();

$car = $carFactory->create();

$car->setEngine($carPartFactory->createElectricEngine())
    ->setWheels($carPartFactory->createGoodYearTires(4))
    ->setStereo($carPartFactory->createBadAssStereo());

You're creating more than one factory to wire together collaborators. Its Builder time, bitches.

$carBuilder = new CarBuilder();

$car = $carBuilder->addElectricEngine()
                  ->addGoodYearTires(4)
                  ->addBadAssStereo()
                  ->build();

Its easier to use, less duplication, easier to read, and if you ever need to vary that creation process its already in place.

Indicator #2: Factory method with optional parameters

Optional parameters are terrible. I think thats a whole post al by itself. For now, if you find yourself doing this...

public function create($first, $last, $middle = '', $prefix = '', $suffix = '')

...do this instead...

$builder->first($first)
        ->last($last)
        ->middle($middle);

Builders in Builders

Final note of the day on Builders. Object chaining can make usage pretty easy. We get use to returning $this in each method. Sometimes though we want the Builder to return another Builder which can configure its collaborator. In this case we return the Builder instead of $this, but its also good to have a method naming convention to let developers know instinctively which they'll get back.

Personally, and I think I picked this up from a book though I can't remember which one, I use "set" or "add", whichever reads best, when I'll be returning $this, and "with" when I'll be returning another Builder. Sometimes I might even have a "set/add" method and a "with" method for configuring the same option, where the "set/add" just uses default settings and the "with" method lets you dig in, but only if you want to.

// using "add"
$builder->addGoodYearTires();

// using "with"
$builder->withGoodYearTires() // returns TireBuilder
        ->setFancyRims()      // call TireBuilder methods
        ->setSnowTires();

class CarBuilder {
    public function withGoodYearTires() {
        // keep reference to builder so we can get its "product" 
        // and add it to the Car we're building
        $this->_tireBuilder = new TireBuilder();
        return $this->_tireBuilder;
    }
}

Stay tuned for Part 4...

Guitar Hero-like scrolling music on Ubuntu

noreply@blogger.com (Bob McKee) — Tue, 02 Aug 2011 13:00:00 +0000

This year I began to learn to play the drums. Its been fun but I am not the most dedicated student. Its a hobby and I slack off a lot, but I really would like to get better.

I first got interested in the drums when I tried them out on Guitar Hero. It was fun playing with friends and you could play for hours without thinking about it. You'd see yourself getting better and more comfortable. You'd notice what got tired or sore after playing for 3 hours so you'd adjust or do some Google-ing and find out how real drummers do it.

Guitar Hero certainly doesn't have the capacity to teach you like a human teacher could. It doesn't how the technique you play with, and that's a biggy. On the other hand, real teachers cost money and scheduled time, which at this point in my life is more than I want to give. Over the weekend I set out on a hunt for something like Guitar Hero with a few extra requirements.

Work with a Alesis DM6 electric drum kit
Runs on Ubuntu
Can load songs of a format which can be easily found for free online (ex. MIDI)
Scrolls music vertically (like Guitar Hero)
Rates your ability to play with the song (like Guitar Hero)

(Note: I realize some electric kits can be connected to Guitar Hero directly but mine cannot. It has a USB-out, not MIDI-out.)

Synthesia via PlayOnLinux

My search brought me through a graveyard of abandoned projects but eventually I found Synthesia. Synthesia, as advertised, is almost what I want. Its actually made for learning/practicing piano, but it works off MIDI tracks, so theoretically it can work with other instruments as well.

Synthesia does not have an official Linux version. There is a forked Linux version, Linthesia, but it doesn't have a 64-bit version and Synthesia's wiki noted it was old and missing features. For a while I dug through discussion forums talking about how they managed to get Synthesia to work with Wine, but experiences seemed to vary and honestly I didn't want to dig in that deep.

Luckily I ended up stumbling upon the PlayOnLinux which, just by chance, added support for Synthesia two weeks ago. PlayOnLinux is a tool which quickly configures Wine for many Windows games and applications. I followed the Natty installation instructions and got PlayOnLinux installed without any issue. I then followed these instructions to install a PlayOnLinux application, in this case Synthesia. Lastly, I tweaked the Wine settings through PlayOnLinux to have Synthesia run in "windowed" mode by following these instructions.

From here Synthesia worked immediately with my wife's MIDI keyboard. Some articles suggest installing Timidity, which may be necessary in some use cases but not for mine. I simply had the keyboard input set to the MIDI keyboard, and the output also set to the MIDI keyboard. I had installed Timidity but it didn't seem to be necessary and it crashed once for me so I removed it.

Is that it for drumming with Synthesia?

No.

This is where I've left off. Synthesia registers my drum hits and can output to my drum kit. I tried downloading MIDI files with percussion tracks and tried playing a long. None of my hits seemed to match the drums Synthesia wanted me to hit. I believe this is a MIDI note mapping mismatch. For example, my snare drum produces note 38, says QMidiRoute. I suspect the MIDI track I was playing to did not have the snare drum sound as being note 38. Overall I'm not really familiar with MIDI at all so this could be a completely ignorant assessment but for now its what I'm working on. When/If I find a good solution I will post it.

PCI DSS compliance and spaghetti code, Part 2

noreply@blogger.com (Bob McKee) — Mon, 01 Aug 2011 13:19:00 +0000

(Be sure to read Part 1 first!)

The first iteration of my Command objects could continue to use our existing payment gateway code, but this would not be acceptable by the end of this project. This old payment gateway code relied heavily on global state to get configuration options and it executed database queries. If that were allowed the database servers which our CDE web server accessed would also be subject to audit. On top of that this code was poorly structured and lacked much of the functionality needed to complete the project.

Completely replacing a fairly large chunk of code is often a bad choice, but we had a mound of reasons for making the move.

Structuring payment gateway code

I worked on this project in parallel with one other teammate, Xiao. While I was tearing through legacy code and building command objects Xiao, wrote all of the new payment gateway code I'm about to describe. I helped him with up-front architecture consulting, code review throughout the development process, and some pieces of refactoring.

Each payment gateway we needed to support had a name-value-pair API (such as PayPal's) or an XML API (such as Authorize.net's). The data used to create each request was usually a combination of several complex objects: merchant account information, name information, addresses, "payment profile" information, etc. These objects created a Composite pattern. We used a Visitor pattern to descend into the Composite and build our final object in the format in which it would be send to the payment gateway.

This might seem counterintuitive because the structure we area creating didn't match their API structure at all. For example, an API that used name-value pairs might include an address. This address wouldn't be a separate structure that, as a whole, is part of our name-value pairs. The address parts (city, state, zip, etc) would be individually mixed up with name, credit card number, expiration date, and so on, because the name-value pair structure is completely flat.

The benefit to having a composite structure with strong typing is that it makes it impossible to make an invalid request. Each structure, like the address, has a set of fields you can define. Every place an address can be added, its specified by type. Passing an object of the wrong type causes an error instantly - do not pass GO, do not collect $200. This kind of defensiveness seems pretty important for a system as critical as this one (one that receives money).

Continue to Part 3!

PCI DSS compliance and spaghetti code, Part 1

noreply@blogger.com (Bob McKee) — Thu, 10 Feb 2011 22:43:00 +0000

Anyone taking a substantial amount of credit card payments will eventually stumble onto the Payment Card Industry Data Security Standard (PCI DSS). Meeting this standard likely adds significant complexity to your application, comes at fairly great expense, and will leave your developers and sysadmins with at least a few head-scratching moments where as they ask themselves "Why do I have to do this? It has nothing to do with security." The project I'm working on has been audited, but the final paperwork is not in yet. Things look good, but pass or fail, there's still plenty to learn from the experience already.

Minimizing the PCI DSS burden

My employer's web application has a fundraising module which needed to become PCI DSS compliant. There are many things we considered when deciding how to modify our fundraising module. Here are a few of the larger concerns:

Hardware which handles credit card numbers is subject to audit (these systems combined are called the Cardholder Data Environment, or CDE)
CDE hardware changes must be documented
CDE code changes are subject to audit
CDE code changes must go through a documented security-focused peer review
Quality testing must be documented for all CDE code changes
There are restrictions on who is permitted to deploy CDE code

As you can see, maintenance to a CDE has lots of extra overhead. It didn't make sense to add that to overhead anything other than our fundraising module so we decided to separate it from the rest of our application code as well as serving it from its own cluster of web servers. This solution also reduces the attack surface area of this sensitive part of the application.

We would obvious have to make some changes that would cost us this extra overhead but we realized we could keep that cost low by reducing the need for change within the CDE code. To do this we tried to keep the CDE code... well... as dumb as possible. Our application has many customizable types of fundraising forms used in various workflows, but we don't want the CDE code to know about that. That stuff is complicated. It changes, grows, and has a higher bug potential than something smaller and simpler. All that complexity should continue to live within our original application which the CDE code can communication with via a simple API. This sounds great, but how do we rip our application in two?

Plotting the course

Segmenting our application this way had huge implications for our existing fundraising code. The fundraising code had feature after feature piled on over 5 or 6 years. The user workflow stumbles through a 5000-line top-down PHP file of spaghetti code. Eventually other "helper" functions and classes might output to browser, redirect, or charge a credit card and then terminated the request with an exit() call. Eww! This code was so complex that it was hard to get a good idea of what all of it did. Without structure or comments it was unreadable and there was no test coverage.

When I first approached this problem I looked at in a very linear way. This is roughly how I saw the workflow for a basic fundraising page with all valid data submitted and no credit card processor errors:

User opens fundraising page on web server (not a CDE web server)
User submits form which posts data to CDE web server
CDE code pulls out credit card number and other sensitive data and validates it
CDE passes the validation results and non-sensitive data to a validate function on the web server API
Web server validates the rest of the posted data using some validation extracted from the spaghetti code
Validation passes so some other extracted code decides to return a "sale command" to the CDE (see Command Pattern)
CDE executes the command
CDE calls web server API telling it the transaction was successful
Web server uses some extracted code to determine if the user is to be shown a "thank you" message or redirected to a "thank you" page (each of which could be different depending on the workflow the user was executing)

All this code extraction was going to be a real pain. The validation and form building was tightly coupled using an old Pear library called QuickForm. The various user workflows were tangled together and extremely overcomplicated. Making a few huge tears in the middle of the application seemed high-risk given our relatively short timeline.

A-ha!

I soon realized this wasn't necessary. I didn't need to do all this extraction. I didn't need to rip this thing apart with a few huge, dangerous tears. What I needed to do was encapsulate those end points which triggered the output of data to the user's browser, a user redirect, or charging a user's credit card.

These encapsulated end points form Command classes which can be serialized and returned to the CDE in an API response. These commands are simple. Its not important what kind of page you are outputting, what kind of page you're redirecting to, or what kind of fundraising charge you're doing. The CDE code doesn't need to know. The CDE code just know how to handle certain sensitive fields, perform a credit card transaction, and execute these generic commands. All other decision making is delegated to the web server API.

For our first iteration I created these command classes and took all that structureless top-down spaghetti code and tossed it into a single class method (we'll call this class a Controller). I extracted global variable usage so their values could be passed in through the Controller's constructor. I also extracted the validation of the sensitive data and had it run outside the Controller with its results injected into the Controller (as would eventually happen with the validation running on the CDE web server). I used several legacy code refactoring methods described in one of my favorite programming books, Working Effectively with Legacy Code. These changes produced something I could release immediately.

Continue to Part 2!

Exceptions as part of "regular" control flow

noreply@blogger.com (Bob McKee) — Tue, 31 Aug 2010 03:38:00 +0000

I've heard this rule a lot: "Never use exception for control flow." Its an interesting statement to parse. We know what an exception is, but the definition of control flow is a little fuzzy, so lets clarify things a bit.

In computer science, control flow (or alternatively, flow of control) refers to the order in which the individual statements, instructions, or function calls of an imperative or a declarative program are executed or evaluated.
http://en.wikipedia.org/wiki/Control_flow

Immediately I'm confused. Is it possible to use an exception and not effect control flow? Nope. Exceptions are a means of controlling flow. Sometimes people making this point specify that its "normal" or "regular" control flow. "Normal" and "regular" are delightfully relative and unhelpful. Normal? Compared to what? What they're trying to get at is that they don't believe exceptions should be used unless there is an application error.

This seems odd considering that business layer classes aren't necessarily made for only one code path or even one application and therefore lack context to determine the exceptionality of the condition. I touch on this a bunch in my Exceptions vs Null article.

There are lots of "crazy" things you can do with exceptions and if you're interested in seeing more check out this article on c2.com. Here I will only be discussing three usages I found particularly interesting.

Exceptions as return values

This example comes from c2.com. I think even people who have never thought deeply about exception usage would never dream up this code. Still, I believe this might be the perfect example of what people are talking about when they say not to use exceptions for control flow.

void search( TreeNode node, Object data ) throws ResultException {
    if (node.data.equals( data ))
        throw new ResultException( node );
    else {
        search( node.leftChild, data );
        search( node.rightChild, data );
    }
}

Get it now? As a starting point I think we can all agree that this it batty. Its a search algorithm that throws an exception upon success. Really? The article correctly points out that this is a violation of the Principle of Least Astonishment. I know this code make me feel violated.

Exceptions as commands to the caller

} catch(MyService_Exception_CouldNotBeReached $e) {
    throw new MyOtherService_Exception_Retry("Couldn't reach my service, retry!");
}

This exception seems to be commanding the caller to retry something. Like the previous example, this also breaks the Principle of Least Astonishment. In order to benefit from the full functionality of the method you need to be setup to catch exception commands that it throws and follow out some other action to continue. This is a application-agnostic service making application-specific decisions (whether or not to retry). No thank you.

Exceptions as loop termination conditions

c2.com offers us another gem, and I don't mean that negatively.

try {
    for (int i = 0; /*wot no test?*/ ; i++)
        array[i]++;
} catch (ArrayIndexOutOfBoundsException e) {}

The first thing when I thought when I saw this was "What about things like Python's StopIteration?" One line later its mentioned. Yay! This article may be reading my mind. I do wonder if the fact that StopIteration exist in Python, Ruby, and Javascript now might start to carve away at exception/null failure/absence debate. I'd like to know more about the decision making that went into the the design of feature but so far have failed to find any discussions on the matter.

All that said, I'm not sure how I feel about the name - "stop" iteration. Sounds like the service commanding its caller. If I were to use a exception-terminated loop I'd prefer to explicitly specify the exception type rather than using a language generic exception with a name that is a command. Maybe something like this (which I would not be surprised to find already exists somewhere):

until(RecordNotFoundException $e) {
    print $recordSet->getNext()->getLabel() . "\n";
}

Categorizing exceptions: Subtypes vs Error codes

noreply@blogger.com (Bob McKee) — Wed, 25 Aug 2010 03:10:00 +0000

Today's problematic pattern: Catching a single, usually per-package, exception and then using an associated error code in a conditional statement to determine what else to do. I've seen several colleague programmers do it and they never seemed to think about doing it any other way. Its an oddly unconscious decision.

Consider the following code:

try {
    $http->get('/~bob');
    // do something
} catch (Http_Exception $e) {
    if($e->getCode() == 404) {
        // do something else
    }
}

When you use things like well-established HTTP status codes this almost seems reasonable. It gets a little weirder when you're talking about some internal code defined in some package no one knows anything about. Take this bank account withdrawal example:

try {
    $customerBankAccount->withdraw(100);
} catch (CustomerBankAccount_Exception $e) {
    if($e->getCode() == 123) {
        // 123 means they overcharged their account, SHIT!
    }
}

My problem with this is that we're using two tools for a single purpose. What is the purpose of an Exception subtype, Http_Exception or CustomerBankAccount_Exception, in these examples? Exception type categorization. What is the purpose of an exception code in these examples? Exception type categorization. Why are we categorizing the same exception using two different systems? Additionally, why would caller code be left to interpret magic numbers like 404 or some other constant value a programmer in your office came up with?

It can get worse. People create their own custom exception error codes with pre-specified ranges like code 100 to 200 are set aside for account balance errors and 200 to 300 are for "the bank spent all your money" -related errors. Once again, can't exception subtypes be used for this? The ranges are essentially more API for a developer to remember.

Better example time - GO!

This is more descriptive...

} catch (Http_Exception_NotFound $e) {

Sure, we all know what a 404 is anyway, but what about a 402 or a 203 or a 912 (Glenn Beck Logic Not Found)? If you're writing this code you're gonna need to figure out what you want to catch. The next guy reading through it probably doesn't want to figure it out and he'd appreciate it if was just looking at some plain English exception class names.

If you want to catch all those 4xx-class exceptions why not...

} catch (Http_Exception_ClientError $e) {

(The official category of 4xx errors is "Client Error")

I bet a lot of developers see this kind of problem and say "OH SHIT OH SHIT OH SHIT!!! I need to write a one line class with no body at all for like 20 exceptions, that's like 20 lines... OH SHIT OH SHIT OH SHIT!!!" I never quite get that. They would rather spend the time writing catch-blocks with if-blocks inside them that each duplicate some evaluation on a magic error code. It won't take very long for that try/catch/if/else boilerplate nonsense to make those 20 lines of exception definition code look mighty appealing.

Look what I found!

During my research for this post I stumbled upon discussions in many programming language communities about this idea that error codes can be used for exception message translations. "That sounds interesting," I thought. I've never considered that use before but as I read through a plan proposed to the Zend Framework I quickly began to dismiss the notion.

Thoroughly enjoy this proposal.

Currently, exceptions can only be handled on a per-class basis, or possibly by string comparison against the message.

Instead, we propose that exception codes be used throughout the framework, using a 4-byte hexadecimal format.

This has at least four obvious benefits.

First, it's now possible to distinguish exceptions based on the type of error instead of only by class. This allows users to handle them intelligently.

Second, codes let us do some interesting things with exception handling. Users would now be able to call a method such as:
if ($e->stringTooShort()) { 
    ... 
} 
if ($e->stringTooShort()) { ... }
thereby making exception special-casing easy.

Third, this gives us the ability to translate error messages using Zend_Translate (loaded only when an exception occurs) and using separate translation files (for example, .mo format). Not all developers speak English; those that do generally prefer to see exception messages in their native language.
http://framework.zend.com/wiki/pages/viewpage.action?pageId=22134

Who cares about point 4, I'm already gagging. Even the language irks me a bit. Just being able to do something doesn't make it "beneficial". A magical genie could come and grant me my one wish; the ability to shit dead baby seagulls. Its certainly something I couldn't do before, but is it beneficial? Maybe. I really hate baby seagulls.

I don't think I have to re-explain my feelings on Point 1 and 2, but whats the deal with lucky number 3? Oh, number 3. If you checked out the article you probably saw that each package in the whole framework has its own error code range preallocated. Its like preemptive namespacing for codes. No, it IS namespacing for codes. Yes, there's already a way to namespace exceptions; the same way we namespace any other class.

If the goal is to be able to translate any exception then we can't really do that with this approach. Other packages using the same error-code-to-string mapping scheme could overlap in code ranges, so we're at least going to have to catch each different base exception type for each package that has its own defined error code ranges. Then we're going to have to somehow map those to different translation files.

I'm not saying its necessarily easier to just map class names to exceptions, but I can't imagine it being harder. This new approach doesn't seem to be solving any problems and its creating a few new ones.

The proposal has tons of user comments. Some people pushed back. One user asked why PHP's Exception class took an error code in its constructor at all (a very good question). One of the proposal's authors answered:

The error code parameter is intended to be used exactly how we're using it: differentiating exceptions within exception "namespaces" (classes).

Sigh.

Finally, translations are for presentation. Are these presentation layer exceptions? No sir, sadly they are not. We're prepping our business layer exceptions with some data that is meaningless everywhere except the presentation layer assuming the presentation layer knows how to interpret the codes in the first place.

I don't know how to end this. I'd like to be shown something that throws my conclusions on its head, but from my reading thus far, I'm not seeing it. Feedback welcome.

Exceptions vs Null

noreply@blogger.com (Bob McKee) — Tue, 24 Aug 2010 19:41:00 +0000

Plenty of developers agree that returning mixed-type results is not a good practice. It leads to conditional statements wherever the method returning the result is used. Everbody agrees thats bad, and then Null walks in, and we're not sure anymore.

Null is suppose to be magical value which can passed as any type of object and represent "no object". Usually methods return Null when the thing we want to get is absent, and that absence is considered normal for our particular application. Maybe the caller asked for a row that doesn't exist in a table or the next line of a file when you've already reached the end.

If the method you are calling may return Null then you must (almost?) always check for Null. Isn't this why we dislike mixed-type return values? This is only the beginning of the problem with Null.

Personally, I've chosen this rule: "In OOP never return Null and instead always use exceptions".

Absence vs Failure

Those whom I disagree with seem to draw the line at "absence versus failure". They interpret the absence of something as being unexceptional and return Null. The failure to be able to do something within a method call is seen as being exceptional so they throw an exception (or maybe they return some other magic value because they think they're mother-fuckin' David Blane, and by that I mean talentless and insignificant).

This excitingly-named article - When To Use Exceptions - asks one question that seems rather compelling and butts heads with the "absence versus failure" rule:

"Who exactly are [library programmers] to decide that not finding [something] is a non-exceptional event for my application?"

I like everything about that statement including the touch of anger. Business layer classes, or libraries, shouldn't be making application layer decisions. Its like a business layer class triggering a fatal error rather than throwing an exception. I determine whats a fatal error, not the tools I'm using to build with. I always separate my application layer from my business layer, but until recently never considered determining exceptional conditions as part of that division.

Suppose we had a web app for managing blog entries. The app chooses a search-engine-friendly /english-words-dashed-together/ URL for the blog post based on the blog post title you entered. This gives us two obvious times were we want to lookup URLs:

We want to use the URL in a new blog post but need to make sure its not already in use (absence is not exceptional)
A blog post URL has been requested and we need to find the blog post and serve it (absence is exceptional)

Both of these scenarios could call upon the same method - $blogPostGateway->getPostByUrl($url). The exceptionality of this scenario depends on the context within the application, yet, as a business class, this gateway knows (or should know) nothing about the application which it lives in.

This does not mean that a method call lacks any context. There are still post-conditions for the method even absent an application context. A method call's post-condition is that it returns a value of a pre-specified type. If an object of the proper type cannot be returned because of absent data then we have an exceptional case. We don't need a new tool, Null, to sit in place of that object because we already have a tool to say "I failed!" - an exception.

Null = repetition

Suppose $blogPostGateway->getPostByUrl($url) does return Null. Now the caller is left to interpret these magic values. Should callers be left to duplicate those conditionals everywhere? That's not good design.

Preferably the "absence scenario" here would be handled on the blog post gateway in a method like $blogPostGateway->blogPostExists($url) which returns a boolean. No Null. No new exceptions necessary. This decision not to use Null also forces the developer to make the right choice and write these extra methods which check for existence.

Whats the difference, really?

This issue is seen by some as a matter of style. Exceptions with try/catch blocks seen as being identical to Nulls with if/else blocks. Is there a difference? I think so the previously mentioned points are huge, but there are functional differences as well.

Exceptions can be handled immediately, eventually, or allowed to go uncaught and fail fast. Null values may not cause problem until much later in a program's execution. That means cryptic error messages and more tracing to get back to where the Null originated.

Is there anyone else who knows a lot about Null that doesn't like Null?

It seems the guy who invented it calls it his "Billion Dollar Mistake".

Summary

Don't use Null. Don't. Do not.

Testing with string objects in PHP

noreply@blogger.com (Bob McKee) — Thu, 29 Jul 2010 23:56:00 +0000

As shown in the last post there are benefits to instantiating the class under test in setUp(). With objects its simple to create them and maintain reference to them so you can change them, but what about things like strings? Unlike in many other modern languages, strings are not objects in PHP. They are not passed by reference. The good news is while strings are not objects, objects can be stings.

class String {
    public function set($str) {
        $this->_str = $str;
    }
    public function __toString() {
        return $this->_str;
    }
}

This class can be used as a string. It can work in PHP's string functions and any other place a string is desired. So pass this to the class you're testing and then change it later. It works all the same.

Its important to note that if you are using these in your test setUp() that you can't do work using that string object during the construction process of the class under test. Initially this slipped my mind as its part of my standard practices. I believe you should never do any work in your constructor at all.

Durable, readable tests

noreply@blogger.com (Bob McKee) — Thu, 29 Jul 2010 23:10:00 +0000

Anyone who has written tests is familiar with our friend the setUp() fixture method. We use setUp() to do all that work that's common across all our tests. It can be used to reduce repetition within our tests, making them simpler, and more readable. Unfortunately testing can get a little more complicated when you need to sense behavior through a dependency.

Take this class below which decides which developer is on-call using a Chain of Responsibility pattern.

class DeveloperOnCall_Bob implements DeveloperOnCall {
    public function getOnCallDeveloper() {
        if($this->_date->inRange('2010-07-19', '2010-07-26')) {
            return 'Bob';
        }
        return $this->_nextLinkInChain->getOnCallDeveloper();
    }
}

A lot of people would just load their test methods with mock object declarations and probably end up with something like this:

class DeveloperOnCall_BobTest extends TestCase {
    public function testShouldReturnBobWhenHeIsOnCall() {
        $date = new Date('2010-07-21'); // date in range!
        $nextLinkInChain = $this->getMock('DeveloperOnCall ', 
            array('getDeveloperOnCall'));
        $nextLinkInChain->expects($this->never())
            ->method('getDeveloperOnCall');
        $actual = $this->_getClassUnderTest($date, 
            $nextLinkInChain)->getDeveloperOnCall();
        $this->assertEquals('Bob', $actual, 
            'Bob should be on call!');
    }

    public function testShouldNotReturnBobWhenHeIsOnNotCall() {
        $date = new Date('2010-07-30'); // date not in range!
        $nextLinkInChain = $this->getMock('DeveloperOnCall ', 
            array('getDeveloperOnCall'));
        $nextLinkInChain->expects($this->once())
            ->method('getDeveloperOnCall');
        $actual = $this->_getClassUnderTest($date, 
            $nextLinkInChain)->getDeveloperOnCall();
    }
    
    private function _getClassUnderTest(Date $date, 
        DeveloperOnCall $nextLinkInChain) {

        return new DeveloperOnCall_Bob($date, $nextLinkInChain);
    }
}

How good is this test? Lots of setup in those test methods. A bit hard to read. The two tests are very similar yet given this design its very hard to reuse that similar code. If the way those dependencies work changes (as they often do during development) both these tests break and need to be fixed separately.

There is another way though. Lets configure our dependencies somewhat lazily. Consider this alternative:

class DeveloperOnCall_BobTest extends TestCase {
    public function setUp() {
        $this->_date =  $this->getMock('Date', array('inRange')),
        $this->_nextLinkInChain = $this->getMock('DeveloperOnCall', 
            array('getDeveloperOnCall')); 

        $this->_classUnderTest = new DeveloperOnCall_Bob(
            $this->_date,
            $this->_nextLinkInChain
        );
    }

    public function testShouldReturnBobWhenHeIsOnCall() {
        $this->whenDateIs($this->duringBobsOnCallTime())
            ->theNextLinkShould($this->neverBeCalled())
            ->andTheDeveloperOnCallShouldBe('Bob');
    }
    
    public function testShouldNotReturnBobWhenHeIsOnNotCall() {
        $this->whenDateIs($this->notDuringBobsOnCallTime())
            ->theNextLinkShould($this->beCalledOnce())
            ->andTheDeveloperOnCallShouldBe('Adam');
    }

    private function whenDateIs($inRange) {
        $this->_date->expects($this->any())
            ->method('inRange')
            ->will($this->returnValue($inRange));
        return $this;
    }

    private duringBobsOnCallTime() {
        return true;
    }

    private notDuringBobsOnCallTime() {
        return false;
    }

    private function theNextLinkShould($expects) {
        $this->_nextLinkInChain->expects($expects)
            ->method('getDeveloperOnCall')
            ->will($this->returnValue('Adam'));
        return $this;
    }

    private neverBeCalled() {
        return $this->never();
    }

    private beCalledOnce() {
        return $this->once();
    }

    private function andTheDeveloperOnCallShouldBe($dev) {
        $this->assertEquals(
            $dev, 
            $this->_classUnderTest->getDeveloperOnCall(),
            "Wrong developer is on call!"
        );
    }
}

This last example may be a but longer than the previous but it seems almost like comparing paragraphs and bullet points in terms of readability. This test is extremely expressive. It is clear and to the point. A person who has never worked with this library can easily see the author's intent.

Finally, if the dependencies change the code that mocks those dependencies is only in setUp() and private methods so the test methods themselves will not need to change.

Faking PHP functions

noreply@blogger.com (Bob McKee) — Thu, 22 Jul 2010 21:28:00 +0000

Usually when I want to fake some kind of functionality provided by PHP, I create a new library that maps all that crazy API stuff baked into PHP to something that makes more sense. Maybe it even uses exceptions rather than -1/0/false/null. Holy shit. I almost never write something that did a straight pass-through - no parameter changes, method signature identical to the underlying function. I'd find myself spending time and thinking a lot about this library I was building. So PHP does it “wrong”, what does “right” look like? This can be a time-consuming exercise, but these kinds of libraries can get reused a lot. Writing tests for what you've done can be a problem as well. One of these problems is the inspiration for this post.

Here's a dumbed-down version of some code I was writing for a MySQL class (Yes, I'm actually writing one of those... in 2010... really!):

class Mysql {
    public function execute($query) {
        $result = mysql_query($query);
        if($result === false) {
            if(mysql_errno() === self::LOST_CONNECTION) {
                throw new MysqlLostConnectionException();
            } else {
                throw new MysqlException();
            }
        }
    }
}

How do I get mysql_errno() to return the code for a lost connection? I should write a class to wrap this MySQL functionality... oh wait... I am. I needed something much simpler. Enter "THE SIMPLEST CLASS EVER!!!"...

class PHP_Functions {
    public function __call($phpFunction, $args) {
        return call_user_func_array($phpFunction, $args);
    }
}

Now we can rewrite the other code by taking the PHP_Functions object in the constructor of our MySQL class and modify two other lines...

class Mysql {
    public function execute($query) {
        $result = $this->_phpFunctions->mysql_query($query);
        if($result === false) {
            if($this->_phpFunctions->mysql_errno() ===
self::LOST_CONNECTION) {
                throw new MysqlLostConnectionException();
            } else {
                throw new MysqlException();
            }
        }
    }
}

In our tests its now easy to mimic these MySQL error conditions. We just have to mock our PHP_Functions object.

/**
 * @expectedException MysqlLostConnectionException
 */
public function testShouldThrowLostConnectionException() {
    $phpFunctions = $this->getMock('PHP_Functions',
array('mysql_query', 'mysql_errno'));
    $phpFunctions->expects($this->any())
        ->method('mysql_query')
        ->will($this->returnValue(false));
    $phpFunctions->expects($this->any())
        ->method('mysql_errno')
        ->will($this->returnValue(Mysql::LOST_CONNECTION));

    $mysql = new Mysql($phpFunctions);
    $mysql->execute('SELECT 1');
}

Generally speaking, if someone shows me a magic method like __call() I find myself unable to reach their heart through their throat but I keep trying. My arms are big and their throats are small. It just doesn't work... I keep “breaking” these people. Anyways, I'm not a fan. I haven't found them to be the least bit useful except for adapting some legacy code (like I am sort of doing in my example) and maybe making a Decorator base class if working with code with no type hints (I don't like those systems either).

There are other options in this case if you find yourself more upset about magic methods than I am. You can move calls to the MySQL functions to protected methods and override them in testing to provide mock functionality. I use this trick a lot when getting legacy code under test (See Working Effectively with Legacy Code. Excellent book.). Write a class like below. Then, for your tests, subclass this, override the methods to provide mock functionality, then run the test on the subclass you created.

class Mysql {
    public function execute($query) {
        $result = $this->_mysql_query($query);
        if($result === false) {
            if($this->_mysql_errno() === self::LOST_CONNECTION) {
                throw new MysqlLostConnectionException();
            } else {
                throw new MysqlException();
            }
        }
    }

    protected function _mysql_query($query) {
        return mysql_query($query);
    }

    protected function _mysql_errno() {
        return mysql_errno(); 
   }
}

I had several cases where I needed to mock consecutive calls so it just seemed like PHPUnit was ready to go with that, so I opted for that route. Either approach would work well.

Code smell: Too many private methods

noreply@blogger.com (Bob McKee) — Wed, 21 Jul 2010 12:46:00 +0000

I mentioned this in my last post so I figured I'd add some examples to clarify. Consider the following code:

class Cache {
    public function set($key, $data) {
        if($this->_useMemcache()) {
            $this->_setInMemcache($key, $data);
        } elseif($this->_useFilesystem()) {
            $this->_setInFilesystem($key, $data);
        } else {
            $this->_setInMemory($key, $data);
        }
    }
}

Its part of a class that would be used for caching data. If you had a view of all the methods on this class there would be groups of similar methods: those with "memcache" in the name, "filesystem" in the name, and "memory" in the name. On any given operation when one "memcache" method would be used other "memcache" methods are used with it. The same would be true for the "filesystem" and "memory" methods.

If this class were around for a while eventually you'd find yourself saying "Damn, I want to reuse this functionality for writing to memcache, but I don't want the whole Cache system."

If this class was built without tests and you ended up with the task to add them you'd be saying "Holy fucking shit of shit, this is fuckin hard to test" in a Lewis Black voice as you wildly shook from side to side. This is your Minnesota winter (google it).

What we really want is separate classes for caching in memcache, the filesystem, and in memory. You could probably wire these together using a Chain of Responsibility patttern. The Chain of Responsibility essentially lets you create a series of fallbacks.

Client: "I need to cache this data."
Memcache: "I don't want to cache this data but I know someone who might. Do you want to cache this data?"
Filesystem: "I'm watching my show, fuck off. Memory, get the fuck in here? Cache this for me."
Memory: "I have no feeling of self-worth and will grow up to be an unsuccessful empty shell of a man, so I may as well comply. OK, filesystem, I'll cache it."

Anyways, once you understand the Single Responsibility Principle you'd never write this Cache class in the first place. Its obviously flawed. Clear as day. Sometime you don't find out until you're already coding though.

The less obvious case

About a year and a half ago I was writing a class that could read and write language translations from a excel file. The class was going to be used to import it into a database or export what was in the database into a spreadsheet. I already had an interface I used to access translations from other sources so I started implementing it. I used a 3rd-party library, PHPExcel, for traversing the grid, reading, and writing.

I was learning PHPExcel's API as I went. We also had somewhat complex schema that the spreadsheet needed to fit. There could be a variable number of columns depending on how many languages were translated, and any number of rows because new strings were always being added or removed.

I got it working but was left with lots of private methods that did what I needed to conveniently on the spreadsheet. I had a good tool to work with (PHPExcel) but I needed more. I should have had a class which used PHPExcel but traversed the data in the way I needed.

Since then our translation files have grown and we found that PHPExcel is a memory hog. Exporting a few hundred (maybe up to a thousand) rows takes several minutes. I've profiled the code and the problem is PHPExcel (FWIW, I hear there are some newer features in later versions that may fix this). Unfortunately, if we wanted to replace PHPExcel with a faster library we have to tear apart the class I wrote because its all cooked in there, hiding complexities in private methods.

Had I separated my concerns properly replacing the underlying Excel reader/writer would be simply a matter of writing a new class with the interface needed to work with my translation importer/exporter class.

Private visibility in open-source

noreply@blogger.com (Bob McKee) — Tue, 20 Jul 2010 21:43:00 +0000

The statement in question: "Private has absolutely no useful role in open-source code."

There's already lots of drawn out discussion about this so I'll keep it short.

Lots of private methods are bad, but not because of OSS. Its a code smell that usually means too many responsibilities (thereby violating the Single Responsibility Principle). If you don't believe me look at the classes in your code base with the most private methods - ugly right?
Private methods are a good way to keep methods short by dividing up work into small well-described chunks. These well-described methods and can be used in place of more heavy-weight documentation in some, but certainly not all, cases. I haven't seen any large scale project with perfect documentation, so its important that the code speaks to you. Self-describing code won't degrade like documentation. (Excellent examples of this in the book Clean Code)
Code that relies on the internals of 3rd-party code is fragile. If the package author changes the internal implementation of a class that you depend on then your shit is going to break. Technically speaking, of course. If you want to extend code for other OSS projects use inheritance or a form of delegate to use the underlying package without having to modify it or depend on its internals.
Plenty of successful programming languages do no have private methods. It can be done. It just requires discipline and technical expertise.

At the end of the day its almost a silly point. Public or private, when talking about these internal-use-only methods programmers should avoid mucking with them. What are you trying to get at anyways? If its something significant it should be extracted into a new class which is in turn made a dependency of the original.

Why I'll never recommend the "Universal constructor" pattern

noreply@blogger.com (Bob McKee) — Tue, 20 Jul 2010 12:44:00 +0000

I read the PlanetPHP feed every day and honestly it usually leaves me scratching my head a bit. Recently I saw a post referencing the "universal constructor" pattern in the PHP framework, Solar. I hadn't heard this pattern name before but I've seen the code a million times looking through other people's code (and maybe even my own 3+ years ago). The people who use it generally think its the best thing ever. It solves that way-too-many parameters problem. In my experience, however, it almost always indicates poor design elsewhere.

The pattern is simple. Constructors take only one parameter ever. That one parameter contains a hash of option/setting pairs (configuration). Here's an example of where you might see something like this...

$userImporter = new UserImporter(array(
    'enable_deduping' => true,
    'email_admin_on_failure' => true,
    'import_batch_size' => 1000
));

Code that uses this pattern is notorious for breaking the Single Responsibility Principle. Breaking this principle leads to tightly coupled, hard-to-test code. Its fairly obvious problem to catch when you think about it. If someone showed you a class with a constructor that had maybe a few required parameters then 10 optional ones you'd probably run for the hills.

My example class apparently reads some sort of importable data structure, knows how to interact with the data's destination, dedupes users, breaks work into batches, sends alert emails, and probably nailed Jesus to the cross.

This approach appears to solve one problem: it gets rid of that long optional parameter list which leads to code like so...

class UserImport {
    public function __construct($enableDeduping = true, $enableEmailOnFailure = false, $batchSize = 1000) {
        // ...
    }
}

$userImport = new UserImport(
    true,  // default
    false, // default
    2000   // custom setting!
);

You're repeating defaults all over the place to get to that last optional value. You can never remember which parameter it is without looking. Is batch size the 5th or 7th param??? Guess I'll go read the code AGAIN.

We can fix this with a better API. Unfortunately the the UserImport example I am giving fails at this point. These parameters shouldn't live together because there are too many responsibilities that correspond to. Here a slightly modified example that might be a scenario where you'd actually use setters for your optional parameters.

class UserImport {
    public function __construct(Framework_DatabaseConnection $databaseConnection) {
        $this->_databaseConnection = $databaseConnection;
        
        // implements logger interface but does not write logs or actually do anything else.  this is my default logger value.
        $this->_logger = new Framework_Logger_Null(); 
    }
    
    public function setLogger(Framework_Logger $logger) {
        $this->_logger = $logger;
    }
}

In this case rather than having an optional logger in the constructor with default value I just set the object's logger to the default, a null object called Framework_Logger_Null. If you want to supply a real logger just use out setLogger setter method.

In summary, constructors take required parameters. Setters take optional parameters.

Next, don't think of using types here. Can't be done. You're writing all your validation in PHP. You could write a base class for that functionality and waste the one chance of inheritance PHP gives you on that. You could couple your class to some validation library you instantiate statically (using "new") or using static method calls. Granted you're on the hook here for scalar types anyways in PHP since it does not offer types for integers, strings, and booleans for example.

In a project like Solar you probably always have good documentation that's kept up-to-date, but for your home-grown PHP project that almost never the case. These things over time always end up with new configuration options added making them even more complex and the documentation more out-of-date. What I am getting to is - how do you know what options are available? Manual documentation works but decays over time. Auto-generated documentation isn't going to help you. Users could just read the implementation of your class to figure it out (I see this way too much). Essentially you are hiding how to use your class. This is true both when you are using these configuration hashes in the constructor and also a method parameter.

The easy way to expose the functionality your classes offer is by exposing that functionality through the public API.

Lastly, although this is on a required symptom of this pattern, I almost never see dependency injection used with this pattern. People don't want to mix simple configuration settings with dependency management because this pattern is suppose to be all about convenience, and how convenient is it to instantiate all kinds of dependency objects when you're creating this object in some client code? Not very. The general solution Universal-constructor people use is to just instantiate dependencies right in the same class, thereby tightly coupling it to those implementations. There are better ways to handle dependency management but thats another post for another day.

Configurable return types kill puppies

noreply@blogger.com (Bob McKee) — Tue, 20 Jul 2010 02:33:00 +0000

Here's some code you could see using PHP's Adodb library. The library isn't the point but its demonstrates a pattern I see a fair amount.

$oldFetchMode = $conn->setFetchMode(ADODB_FETCH_ASSOC);
$recordSet = $conn->execute("SELECT * FROM table");
$objectThatDoesWork->work($recordSet);
$conn->setFetchMode($oldFetchMode);

To me this is a multi-fail.

If an exception is thrown before the fetch mode is reset it doesn't get reset unless you catch it. That can add up to a lot of boiler-plate try/catch blocks.
If the next call to execute() doesn't set the fetch mode first in order to guarantee you get the return type you expect bad things can happen. I've seen plenty of code that doesn't explicitly set the fetch mode every time and it works... until it doesn't. Code somewhere else in your system changes the fetch mode. The error is coming from code that source control tells you hasn't changed recently. Your tests prove to you that the code works. Your query logs show you that the query you expected to be run was actually run. It sends you in the wrong direction every time.
Setting the fetch mode every time with this long method and verbose constant adds a bunch of boilerplate.
$objectThatDoesWork->work() should also have its first parameter be typed or you'll only know something went wrong when it tries to access that first row the wrong way rather than failing immediately. I'd prefer it to fail fast (pdf).

At first glance it could look flexible and convinient. You don't need to set the fetch mode ever in theory. You can just use the default, and that's pretty simple right? I'd be happy to rip this method out and pretend it never existed, but it does and its scaring the kids.

Luckily, there is actually a easy enough solution to this problem. Take a look at this rewrite.

$recordSetFactory = $conn->execute("SELECT * FROM table");
$objectThatDoesWork->work($recordSetFactory->asAssoc());

$recordSetFactory has the underlying record set resource stored privately. $recordSetFactory object might have different ways to access the data such as asAssoc(), which would return an iterator where the current row is returned as an associative array. It could have another that returns an iterator which returns row data with numeric indexes (asList() maybe). There are plenty of ways to access this data conveniently.

Best of all that configuration is explicitly requested every time. Its used and it gets thrown away when the object's lifecycle ends. Done and done.