Unit testing headers in PHP

There's plenty of questions about unit testing code that sets headers using PHPUnit. PHPUnit officially assumes that neither the test code nor the tested code emit output or send headers:

(from https://github.com/sebastianbergmann/phpunit/issues/720#issuecomment-10399612)

And rightfully so. Headers are essentially global state, which is hard to unit test. However, if you're willing to install the PECL extension Runkit, you can actually unit test headers if you really want to.

I use the setupBeforeClass hook to only run the code once before the entire class runs. If you only have one method that you're testing it can all get stuck in the single test. I check if the runkit extension is loaded to avoid breaking the unit test suite if someone tries to run it that doesn't have runkit installed. The test that actually looks at the headers is marked with a requires annotation, which will mark that test as skipped if runkit isn't loaded. If every test in your test class tests headers, you can move that up to the class level if you'd like. My particular class had some tests that didn't need runkit and I wanted those to still run for people that didn't have runkit loaded.

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.

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.

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.

Building rock solid software in the real world

http://www.flickr.com/photos/preef/32995286/

Recently (2011-11-08) I gave a talk at the Dallas PHP meetup about building rock solid software as a team. For my first experience talking in front of a crowd since high school, I thought it went pretty well. Several people have asked for me to post my slides (which I did), but they were made in a way that doesn't really help people out if they didn't see the talk. The talk was recorded and is available on Ustream, but I thought it might be helpful to do a blog post on the topics I covered as well.

This post is mainly meant to aggregate links to the topics that I talked about.


Tools


I covered several tools. All of these should be available to the developers as build targets and run in your continuous build. Lint and your unit tests should be run as part of your submission process.

• lint - The bare minimum, it just detects syntax errors in your scripts. Code that doesn't pass the lint test won't pass any other tests or manual QA.
• PHPUnit - Standard unit testing framework. There is plenty of information about it elsewhere.
• PHP Code Sniffer - Detects code smells that should be fixed. Many bad programming practices can be written as "sniffs" along with most rules from your smile guide.
• PHP Mess Detector - Statically analyzes your code for possible bugs or coding practices that tends to hide bugs.
• PHP Copy Paste Detector - Scans your code to find large similar blocks which can be factored out to a common method.
• PHP Dead Code Detector - Scans your code to find code that can not be reached. For example, code after a return statement.
• Code coverage - Adding the xdebug extension to your system allows PHPUnit to calculate how much of your code is run by unit tests.

Code reviews

I talked about two different code review packages:

• Review Board - You install it on your server and host it yourself.
• Codereview.appspot - Use Google's code review tool.

And I talked about three different ways of doing reviews:

• Pre-review - Code doesn't get submitted until a peer reviews it. Keeps bad stuff out of your code base.
• Post-review - Code gets submitted, then gets peer reviewed. Comments made about the code may never get resolved, but code reviews don't slow down getting code into production.
• Public shaming - Put the code up on a projector and discuss as a team. Great way to destroy programmer morale.

I mentioned a few points about what to look for in a code review:

• anything the tools couldn't catch
• logic errors (like ifs that don't make sense)
• loops with off-by-one errors
• performance problems (SQL in a loop)
• things to refactor (large methods)
• or things they missed
• Style problems (not really wrong, but you know, wrong)
• Typos (variable names, documentation)
• Tests that don't have assertions
• Methods without tests

Style guides

There's two ways to choose a style guide:

1. Roll your own - Look at your existing code and build the style guide from what you're already doing.
2. Use existing - Such as Zend or Pear.

How to Destroy Productivity on Your Programming Team

Coding on a team of developers is challenging. You've got different coding styles, different personalities, different strengths, and different weaknesses. Aligning all of the team members together can be extremely difficult, and can result in some pretty amazing things when you get it right. But it also results in higher expectations for the future. If you over deliver, you're expected to do the same in the future.

Here's how to make sure that doesn't happen:

• Treat members of your team differently. Only allow certain members of your team to work remotely. If a team member is used to working remotely from previous employment, make sure that he is required to do all of his work at the office. This ensures that he puts in only eight hours a day and can't maintain focus on projects during off hours.
• Make sure there are plenty of interruptions in your developers' office. This is easy to do if you have a cubicle environment, since there is no way for any team member to find silence or privacy. If you do have offices, be sure to cram several developers in to an office. For even better results, make sure that developers that work on the same project do not share offices with each other. Best results can be achieved by seating developers with non-developers that have lots of phone calls and in-person discussions. Salespeople and customer service representatives are perfect for this. Cubicles provide yet another benefit in that they transfer vibration very well. One team member's finger drumming can completely disrupt concentration of others.
• Don't use source control. Allowing developers to use source control keeps them from stepping on each other's toes. It also keeps a record of changes so that the source of bugs and regressions can be found.
• Avoid unit testing. Unit testing helps avoid functionality regressions and new bugs. While it seems that doubling the amount of code developers write would be good for destroying productivity, the gains that come from good unit testing should be avoided. However, you can still impede productivity by halfway using unit testing. This means having one team member write and maintain unit tests for the entire team. Other team members should never run the unit tests.
• Develop an in-house coding standard different from all other coding standards for the language you program in. This ensures that automated code formatting and linting tools are at best difficult for the team to use.
• Ignore established testing and design patterns. Using common idioms makes it easier for developers to keep everything in their head.
• Think about using one of the new software design methodologies. Agile and scrum are very popular for development teams. You should think about using them, maybe have some meetings to discuss the concepts and benefits. Under no circumstances should you actually try them. It is relatively easy to find excuses not to use them.
• Switching the task that individual members of your team are working on helps them to be more well-rounded developers. Allowing them a sufficient block of time to finish a project would only let them gain satisfaction in some finished code. For best results, constantly switch their top priority so developers leave a long line of half finished projects in their wake.
• Make sure that developers do not have adequate machines to do their work. Developers like to have many applications open at any given time. They should have to shutdown Eclipse to load Photoshop for example.
• Impose restrictions on the tools that developers use. Developers get used to using certain programs. Require all developers on your team to use Emacs, or Vim, or Eclipse, or Netbeans. This is also a great way to kill some productive time by arguing about which development environment the team should use.
• Have lots of meetings spread throughout the day. Meetings are a great time sink. In addition to the time actually spent in the meeting, developers may have to prepare for the meeting. To amplify the effect of your meetings, make sure they are spread throughout the day. Developers work best in large blocks of time. So instead of putting all of your meetings in one contiguous morning block, spread them throughout the day so developers will have 45 minute pieces of development after taking in to account commuting to/from meetings, replenishing caffeine, and using the restroom.

Do you have any other ways to subtly impede the productivity of developers?