MidwestPHP Review

Update: Despite otherwise having a pleasant experience speaking at the conference, things have taken a turn for the worse. It's been almost two months since speaking, and even longer since I purchased flight tickets to go to Minnesota to speak. The organizers seem to feel that the speaker package is more of a general guideline than an actual agreement. Apparently, several speakers, including me, have not been reimbursed. So even though it was a good conference to go to as an attendee, I can't recommend anyone volunteer to speak at it.


It seems like smaller regional PHP conferences are popping up all over the place these days. While they don't have the budgets of the bigger conferences like ZendCon and |Tek, they do provide something those other conferences just can't: an intimate setting. The newest conference on the block is MidwestPHP. I've never been to Minnesota, but submitted two talks to them anyway. Both somehow got accepted and I had a blast at the conference, even while speaking (I'm terrified of public speaking, but it is getting easier). Here's some thoughts on the conference:

The Good

• Organization - Many attendees that I talked to had no idea it was a first time conference run by first time organizers. Things went relatively smoothly. Kudos to Mike and Jonathan for a great job.
• Speaker gifts - As a fan of craft brews, I appreciated a four pack from a local brewery.
• Newer speakers - Smaller conferences with four tracks have space to take a chance on some new speakers, injecting some new blood into the speaking circuit. The PHP community seems to have the same speakers at many of its conferences. Midwest did a good job of mixing "big name" speakers (like Chris Hartjes) with relative newcomers (like me) as well as some first timers. Giving new people experience is great for the future since the larger conferences might feel safer accepting a talk from someone that has spoken before.
• Attendees - It's great seeing so many people so eager to soak up knowledge. I made note of a few people at the beginning of the conference and mentally stalked them throughout. Unlike many conferences I've been to, they weren't always with the same group. Being willing to talk to random people at a conference is a great way to amplify your benefit from going to a conference.
• The venue - Some intimate rooms as well as larger rooms, all with tables. Lots of wide hallways for the important between session discussions.

The Less Good

• Speaker gits - While I appreciate good beer, some people don't drink. And based on the four 4-packs I ended up with, some speakers weren't interested in carrying them home.
• The venue - The rooms were very spread out and difficult to find at first even with the map. More signs directing people where to find where they're going would be very helpful. Also, as with most conference rooms, more power would be awesome. 
• No organized social - There were a few attendees that I would have liked to socialize with after the conference finished for the day, but with no social event to go to they just went home. So it ended up being just a bunch of the speakers socializing with each other. While the other speakers are cool and all, I already knew several of them. I'm not implying that the social needs to be a huge party, but just reserving a large room at a nearby restaurant would be cool.

The Ugly

• The cold - I hate it. My thin Texan blood still hasn't warmed back up to my normal body temperature. 
• The snow - Delayed some flights and froze me even more. I admit it, I'm a wimp when it comes to the cold.
• My talks - I will not be submitting talks to any more conferences. I enjoy being a speaker, but absolutely hate the insane amount of preparation it takes to give one decent talk, much less two. While public speaking is something that I feel like I'm getting better at and have shed most of my fear for, it's still not something that I enjoy doing. I'll still speak at conferences I've already submitted to, if they pick my talks, but I won't be crafting any more submissions.

The Sessions

As for the sessions, with four tracks I obviously missed more sessions than I attended. Some sessions were easy for me to pick, like the ones where I was speaking. Some were easy for me to decide not to go to, like Chris's testing talk which is awesome, but I've seen it. There was a good mix of basic, intermediate, and advanced talks as well as a good mix of front-end and back-end talks. The talks I personally really enjoyed:

• Scaling PHP with HipHop by Sara Golemon - Since the first time I heard about the HipHop project I've been fascinated by it, even though I never really had a reason to use it. Her talk was interesting from a historical and architectural perspective. Again, I'm probably still not going to have an excuse to run HipHop in production, I do plan on playing around with it.
• Caching and Tuning fun for high scalability by Wim Godden - Great pointers for when and how to use caching, with some real world examples. Much of this would be immediately useful to sites trying to scale up.
• Embrace Your Inner Designer by Josh Broton - This was a stretch for me, since I'm about as far from a designer as they get (I don't "do" pretty). But I actually feel like I gained something from it to make me a slightly less shitty designer. And Josh probably had the best delivery of any speaker that I saw. Just don't tell him I said that.

Overall, great conference. I learned a lot, met some great new people, and had a great time. I will probably make the trip up next year as well, though I may need a heavier ski jacket to protect me from the sub-zero temperatures.

Unit testing code that uses static methods

Static calls are death to testability. There are ways of mocking out static calls made inside a class, but if you have some code that makes static calls outside a class, it's much more difficult. The best way is to refactor the static class to an instance class so you can mock things out.


But if you're working in a legacy code base with large and ugly static classes, refactoring them may be overly difficult or hard to justify from a business perspective. But you still want to test the new code that you're writing that uses those static classes. A coworker showed me one way to do it. It's still hacky and not as good as fixing the static class properly.

Basically instead of refactoring the crufty static code, you do the refactoring in the class you're writing. Here's an example class that uses the static Calculator class from above:


You can't mock out the Calculator class in this case. But you can make changes to your Foo class that will let you control the output from the static class. You can add protected methods to your class that call the static methods, and then mock those methods for testing and test the mock of the class you're testing.


Obviously this example is very simplistic, but in a more complicated class you could have the static class throw exceptions or different output or whatever.

Now you have two problems

This technique is far from perfect. If you were to change the behavior of the protected methods of your class, the unit tests would still pass, but the code would no longer work the same in production.

Unit testing Google Closure applications from the command line

I've been playing around with building an application using Google Closure. I tried searching for a good way to run the unit tests from the command line as part of an Ant build, but either there wasn't anything out there to do what I wanted to do or I just couldn't find it.

As an aside, naming a language after a programming construct is really dumb. If you try searching for blog posts on Google Closure, you get a bunch of stuff about javascript and closures since they're such an important language concept. I think Prototype might have been a more successful language if they hadn't named themselves after a language construct as well, though the framework itself has its issues.

I'm used to writing code using test driven development, which doesn't work particularly well writing some Javascript. DOM-related code in particular is difficult to write with TDD. But there are parts of Javascript applications that can and should be well unit tested. When coding in PHP or Python, I normally have two terminal windows open for my Vim sessions and one for my build and source code management activities. I'll typically save the file I'm working on and them immediately alt-tab to the build window and hit up then enter to run my unit test target. I really wanted to do that with Javascript.

At my job we use jQuery. This comes with Qunit which is easy to run automagically with PhantomJS. But Qunit didn't look like it would be easy to make work with Closure in that it wouldn't handle the dependencies for me. And I might as well use the unit test framework for the library that I'm working with. I had a few goals. I didn't want my Javascript tests to fire up a browser. That kind of thing should be handled by Selenium. I wanted to be able to add new tests without having to manually add the test name to any file. I wanted the output to be at least somewhat pretty and clean, particularly if there were no failures. As a PHP developer I'm used to the PHPUnit output:

I decided to try to code up my own test runner to make this work. Here's the first version:


First, the build target concatenates all of the test files. In my case, test files are in a directory called tests and each of them ends with Test.js.

Next, it fires up the Closure Compiler. In this example, we've got a Javascript file called foo.js. We compile that with the concatenated test file (tests-concat.js) and the test runner (we'll get to that soon). The compiler creates a file called tests.js. We run that with phantomjs. The build will look something like this:

Most of the magic happens in testRunner.js:

Basically how it works is to override some methods in goog.testing.TestCase to capture the results. We don't particularly care about successes, so they're replaced by dots. Failures show as an F and any errors are explained in more detail after all tests finish.

I've uploaded all of the example code to Github at https://github.com/omnicolor/Closure-CLI-test-runner.

One assertion per test

There's a right way and a wrong way to write unit tests. I've seen lots of wrong ways (like unit tests with no assertions) and differing shades of rightness. One thing that isn't really "wrong" but is not as right is writing your test cases with multiple assertions per test case.

Here's a simple example of a test with multiple assertions:

function testSomethingThatReturnsAnArray() {
    $value = somethingThatReturnsAnArray();
    $this->assertEquals('Foo', $value[0]);
    $this->assertEquals('Bar', $value[1]);
    $this->assertEquals('Mitz', $value[2]);
}

This could easily be rewritten to multiple assertions:

function testSomethingThatReturnsAnArrayFirstElement() {
    $value = somethingThatReturnsAnArray();
    $this->assertEquals('Foo', $value[0]);
}

function testSomethingThatReturnsAnArraySecondElement() {
    $value = somethingThatReturnsAnArray();
    $this->assertEquals('Bar', $value[1]);
}

function testSomethingThatReturnsAnArrayThirdElement() {
    $value = somethingThatReturnsAnArray();
    $this->assertEquals('Mitz', $value[2]);
}

What does this gain? You can immediately see whether somethingThatReturnsAnArray() is broken for all of the tests or just the first. With the first example, if $value[0] is not 'Foo', the test will immediately fail and you won't even test whether the next two elements are correct or not.


But the method I'm testing is slow!


If somethingThatReturnsAnArray() is so slow that you don't want to run it three times, you've got two options:

• Make the method faster.
• Only run it once per test suite run.

Obviously making the method faster would benefit your code the most overall, but it isn't always possible. But you can always just run it once, either by saving its output as a member variable of the test suite or making the tests depend on each other.

Saving the output is easy:

$result = null;
public function testSomethingSlowThatReturnsComplexResult() {
    $this->result = somethingThatReturnsComplexResult();
    $this->assertEquals('Foo', $this->result[0]);
}

public function testSomethingSlowThatReturnsComplexResultSecond() {
    $this->assertEquals('Bar', $this->result[1]);
}

But there is a problem. Tests shouldn't require a certain order unless it is explicitly set. If you move the second test method above the first it will start failing. You could have a helper method that only executes the function once:

$result = null;
private function runSomethingSlow() {
    if (!$this->result) {
        $this->result = somethingThatReturnsComplexResult();
    }
    return $this->result;
}

public function testSomethingSlowThatReturnsComplexResult() {
    $result = runSomethingSlow();
    $this->assertEquals('Foo', $result[0]);
}

public function testSomethingSlowThatReturnsComplexResultSecond() {
    $result = runSomethingSlow();
    $this->assertEquals('Bar', $result[1]);
}

Now the slow function only is executed a single time, and the test methods don't have an unnamed dependency on any other tests.

But my method is really complicated!

If you have a very complicated method that has lots of dependencies, you may be tempted to test actions on all of the dependencies at the same time as well as the return value so you don't have to have all of the boilerplate code repeated. I've heard some otherwise very talented coders make this argument, which always surprises me. Test code is not really different from production code in that if you find that you're repeating code several times, it should be factored out into a method.

Let's try testing everything about this method:

function compareAppleToOrange(Apple $apple, Orange $orange) {
    return array(
        'sizeDifference' => $apple->getSize()
                - $orange->getSize(),
        'peelDifference' => $apple->getPeelWidth()
                - $orange->getPeelWidth(),
    );
}

You could test it all in one big test:

function testCompareAppleToOrange() {
    $apple = $this->getMock('Apple',
        array('getSize', 'getPeelWidth'));
    $apple->expects($this->once())
        ->method('getSize')
        ->will($this->returnValue(4));
    $apple->expects($this->once())
        ->method('getPeelWidth')
        ->will($this->returnValue(2));

    $orange = $this->getMock('Orange',
        array('getSize', 'getPeelWidth'));
    $orange->expects($this->once())
        ->method('getSize')
        ->will($this->returnValue(3));
    $orange->expects($this->once())
        ->method('getPeelWidth')
        ->will($this->returnValue(3));

    $expected = array(
        'sizeDifference' => 1,
        'peelDifference' => -1,
    );
    $result = compareAppleToOrange($apple, $orange);
    $this->assertEquals($expected, $result);
}

Better yet, you can test each piece on its own:

function testCompareAppleToOrangeSize() {
    $apple = $this->getMock('Apple',
        array('getSize', 'getPeelWidth'));
    $apple->expects($this->once())
        ->method('getSize')
        ->will($this->returnValue(4));
    $apple->expects($this->any())
        ->method('getPeelWidth');

    $orange = $this->getMock('Orange',
        array('getSize', 'getPeelWidth'));
    $orange->expects($this->once())
        ->method('getSize')
        ->will($this->returnValue(3));
    $orange->expects($this->any())
        ->method('getPeelWidth');

    $result = compareAppleToOrange($apple, $orange);
    $this->assertEquals(1, $result['sizeDifference']);
}

function testCompareAppleToOrangePeel() {
    $apple = $this->getMock('Apple',
        array('getSize', 'getPeelWidth'));
    $apple->expects($this->any())
        ->method('getSize');
    $apple->expects($this->once())
        ->method('getPeelWidth')
        ->will($this->returnValue(2));

    $orange = $this->getMock('Orange',
        array('getSize', 'getPeelWidth'));
    $orange->expects($this->once())
        ->method('getSize')
        ->will($this->returnValue(3));
    $orange->expects($this->any())
        ->method('getPeelWidth');

    $result = compareAppleToOrange($apple, $orange);
    $this->assertEquals(-1, $result['peelDifference']);
}

This allows you to change the peel calculation without touching the test for size, or the size calculation without touching the peel calculation.

If you wanted to really split the test up, you can go even further since the mocks  provide some assertions of their own with the expects() calls. We'll split up the sizeDifference tests:

/**
 * @return array Array from apples to orange comparison.
 */
function testCompareAppleToOrangeSizeCalls() {
    $apple = $this->getMock('Apple',
        array('getSize', 'getPeelWidth'));
    $apple->expects($this->once())
        ->method('getSize')
        ->will($this->returnValue(4));
    $apple->expects($this->any())
        ->method('getPeelWidth');

    $orange = $this->getMock('Orange',
        array('getSize', 'getPeelWidth'));
    $orange->expects($this->once())
        ->method('getSize')
        ->will($this->returnValue(3));
    $orange->expects($this->any())
        ->method('getPeelWidth');

    return compareAppleToOrange($apple, $orange);
}

/**
 * @depends testCompareAppleToOrangeSizeCalls
 * @param array $result Result of comparison.
 */
function testCompareAppleToOrangeSize($result) {
    $this->assertEquals(1, $result['sizeDifference']);
}

Each split gives you more information on a failure and makes debugging test failure easier as well as reducing the changes required for refactoring.

What's in a (method) name?

Through my coding career I've come with some signs that the method I'm writing or reviewing probably needs to be refactored. These aren't hard and fast rules, but in general these are sniffs that something may be amiss.




Methods with 'or' or 'and' in their names


It's a pretty good sign that you're doing too much in a single method if it has the word 'and' in its name. For example, addMessageAndNotifyUser() should probably be split into two methods: addMessage() and notifyUser().


Methods with 'or' in their name aren't as easy to classify. If the whole method is just a single conditional, the method name may make perfect sense. I would argue that it is just poorly named.




Methods with almost the same name


If you've got multiple methods that all have basically the same name except for a useless word, you're probably making the code harder to understand. For example if you wanted to add a message, which of these would you call?

• addMessage
• addMessageProper
• addMessageActual

If the first addMessage calls one of the other ones, is adding a message its main task? Could it be renamed to something that makes more sense?




Methods that start with 'do'


Do is always a wasted word. I see this mostly from coders that got their start with Visual Basic. Just don't 'do' it.




Why does it matter?


Readability is the most important thing to strive for in code. Bug free, maintainable, and fast code come from code that is readable. Most of the code's life cycle is spent in maintenance. The longest part of a typical maintenance task is to figure out what a particular piece of code is trying to do. Good documentation goes a long way towards making code understandable, but if the methods are actually well named you won't even need to look at the documentation.




Update (2012-03-15): I felt the need to revisit this post. Poor naming in the code base I'm working on wasted several hours of my day. I was trying to find where a set of objects was getting loaded from our data store. Looking through other code that did the same type of thing was a deep rabbit hole of included files and static calls to other classes. Finally I asked a teammate where the objects get loaded. He pointed out that they were loaded in a method named 'handleSortingAndPagination'. The method name doesn't provide any hint that it loads the objects from the data store before sorting and paginating them. The documentation for the method didn't either. This adds another bad naming smell: names that do not reflect what the method does.

Consistency is the key

http://www.flickr.com/photos/richard-g/3549285383/

In my last post, Keeping it simple, I wrote about a few things that can make you a better coder, or at least a more valuable member of a coding team. This is the next step down the path of coding nerdvana.



Style isn't just for the stylish


Every coder has their own preferred style. Left to our own devices, we tend to write code our own way. As long as you're the only one looking at the code, this isn't a problem, but consistency should still be valued.

If you're part of a team, you hopefully have a well documented style guide that everyone follows. Hopefully it covers the gritty details so that developers don't get into fist fights with each other about style differences. The last thing that a growing code base needs is for you to be able to tell who wrote a piece of code without checking your source control's blame log.

But what if it doesn't cover a style point?

Stay consistent!


Perhaps it's my upbringing as a military brat or my overly-logical thought process, but I just can't handle disorder in code. It seems so simple to me, but I see code like this entirely too often:

function foo($bar, $mitz) {
    if ( 0 == $bar ) {
        doSomething();
    }
    if($mitz == 0){
        doSomethingElse();
    }
}

There are some very valid reasons to write your if conditions one way or the other (0 == $bar instead of $bar == 0). I'm sure there are people that will make arguments about whether to put spaces outside of the if condition parenthesis, or extra spaces inside them. But doing it two different ways in a single method is just crazy.

Mixing styles in your code makes it an order of magnitude more difficult to read. Code that is hard to read is hard to maintain. Your code will spend more time in maintenance than in development, so why wouldn't you do everything you could to make it easier to maintain?

Keeping it simple

http://www.flickr.com/photos/r_rose/102766969/

I make no claims at being really smart. I don't even claim to have above average intelligence. But I have worked at companies that have a higher than average bar for employment and consequently above average employees. And working with really smart people, I've noticed that they tend to make one common mistake:

They write really complicated code.


A lone wolf


If you're a single developer working on a project, complicated code might be okay for you to write. Assuming that you write good documentation and are as smart as you think you are, you can write some really clever code. If you are truly as smart as you think you are, you can then maintain that code when you come back to it later. My experience with my personal projects has led me to a few realizations:

• I'm not as smart as I once thought I was
• I'm not as good at writing documentation as I thought I was
• My unit tests aren't as clear as I thought they were

Again, you may be smarter than me, but you're probably not as clever as you think you are.

Joining the pack

Now, join a team of developers. You've got a group of people with varying skills and experiences. None of them are as smart or clever as they think they are. If you can't even understand the complicated code that you wrote as a team of one, how likely is it for the rest of your team to understand your code?

You're no longer writing code in a vacuum. It almost immediately becomes impossible for any team member to understand how the whole system works as the system becomes more complicated. So each developer has little fiefdoms that they wrote, and since they're trying to impress other developers they make sure their intelligence shows through in the code.

Now, join your team with other teams in the workplace... You see where I'm going with this?

Your sanity went that way

The solution is to keep it simple. Assume that when your code breaks, you're going to be expected to fix it on a Friday night after drinking a dozen beers or at the darkest part of the morning when you've run out of coffee. The last thing you want is to have to figure out what your code is doing before you can fix the problem. That means write more documentation about how the code actually works and what it is actually doing. It means avoiding anything that makes it more complicated than it needs to be.

Things to avoid in your simple code:

• Big methods - They're hard to write, hard to test, hard to debug, and most importantly hard to understand.
• Magic numbers - If you don't immediately know what a number means by looking at it, it should be replaced by a constant. And even if you know what the number means, does everyone on your team know? Many coders know that there are 86,400 seconds in a day, but that doesn't mean it shouldn't be replaced by a constant.
• Conditionals - Sure, you're going to need if statements to write a decent sized program, but each branch your method has increases its complexity. You can have a small method that is extremely hard to understand if there are many branches.
• Planning ahead - Programmers tend to be lazy. We try to think of every possibility ahead of time and program for things that may never happen. We needlessly complicate simple code thinking we can see into the future. And if that future never happens (more likely then we would like to admit) the code is wasted. And worse then wasted, it's difficult to understand. Since the code is only supposed to do one thing but you've coded it to do three, maintainers will assume that the three things it does are all equally important.
• Bad names - As part of the growing complexity, it's easy to throw an extra bit of functionality into an unrelated method. Suddenly your simple method sendMessage(), which should just send a message from point A to point B can send a message or log you out of an application or change a configuration option. But if you're not intimately familiar with the code, you naively assume that the method just sends a message.
• Static functions - Static classes and methods look great. You can call them from anywhere, and you can consolidate the similar functionality into a class. You can even unit test the heck out of that static class. But they rapidly increase the complexity of your lower-level code. They easily allow you to include huge chunks of functionality all over your application just by making a static call. That sendMessage() function needs permission, so it's easy to add a SecurityHelper::hasPermission() call inside sendMessage(). Suddenly, sendMessage() doesn't just send a message. It really becomes sendMessageIfSecurityHelperHasPermissionSaysSo().

Writing lots of documentation, adhering to a style guide, and doing test driven development can help keep your code simpler. You'll thank yourself later, trust me.