Chris Holmes Online

“You need to unit test your emails”

Comment from a coworker, proofreading another coworker’s email, after discovering grammatical errors that were the result of a long day.

Recently at work I was asked to review some code with a coworker who was concerned with the way her code look. It didn’t seem “right” to her, and she wanted to me to look at it and see if there was a better way.

The code had unit tests and FitNesse tests and the code coverage statistics revealed that it was covered 99%. It was a complex piece of business logic and it did what it was supposed to do according to the tests; they all passed. But still, my coworker didn’t feel like she had written it as well as it could be written.

“It seems too procedural,” she said. So I took a look.

It was a single class that consisted of one method; it’s job was to perform calculations on some values on a given business object. The method consisted of 433 lines of code. Sure, it was broken up into smaller private methods that were a bit more descriptive, but it was still 433 lines of code for a single method call. And there were comments littered throughout the code informing the reader of what it was actually doing. See, despite the dozen or so private methods that had descriptive names, you still couldn’t follow it by just reading the code. It was too complex, a by-product of the code being procedural.

The length and complexity of the code wasn’t the only smell. The unit tests gave off their own oder as well. Some of the unit tests were as many as 30-40 lines of setup code; tons and tons of mocks just to test one path through the code.

To my coworker’s credit she recognized the deficiencies in her code. She knew it was too procedural, she knew the tests were too big and required too much setup, and she knew that the entire solution needed to be more object oriented.

She just wasn’t quite certain how to get there.

We started refactoring by first asking the question: What is this method doing? What is its responsibility? At first glance, it looked like it was supposed to do one thing. But when I prodded my coworker further we both realized it was actually doing six different things. Six different things in one method; that was too much work.

So we left her code behind and started with a new concrete implementation of the main class, implementing the interface that the original class implemented. We began by asking questions about what each responsibility inside that giant method call was, and what might a class look like that would implement just that single responsibility. The outer class and the single method call would then become a simple coordinator between all the smaller classes.

When we finally started coding, we creating those classes, test first. Each little class got its own test fixture and group of unit tests. The logic in the 433 line of code method started to get pushed off into he smaller objects. Since they were small classes responsible for a single responsibility, the code that got pushed off actually became smaller. There was less to do – less branching – so the result was a tight unit of code that did one thing and did it well.

Slowly, piece by piece, bit by bit, the giant 433 line method got broken up and reformed into a new method that called the tiny classes where appropriate.

We didn’t finish the entire refactoring that day, but we were well on our way to a much better design. And the process was a great learning tool. My coworker got to see how procedural code could be turned into an object oriented design, and how certain small classes could be reused in the larger algorithm.

At the end of the day my coworker turned to me and said, “So I guess if a method is larger than my screen, I need to rethink it, huh?”

And I thought: that’s a great rule of thumb. It is one that I have followed for quite some time in a sort of instinctive way. If I find myself writing a method that ends up being larger than my screen in Visual Studio, I know there’s a chance to refactor to a better design. Methods shouldn’t be that big.

When we left work that day I felt a great sense of accomplishment, and I know my coworker did too. She learned a lot, and our code quality improved significantly. The FitNesse tests will still pass when we complete the refactoring, so the end result won’t have changed much. But a big win still occurred: our design was better and would be much more maintainable moving forward.

I got into software development because I wanted to make video games. That didn’t work out, but I stayed in software development because I enjoyed the idea of being able to enact immediate change in my work. Software is soft, and that means we can improve it today, tomorrow, next week or next year. And we can see the results of our improvements immediately; we don’t have to wait for the cement to harden, we can just hit “Build”.

But quite often there are barriers to enacting change and improving design, and often those barriers come in the form of customers.

Agile development teaches us to be flexible: we utilize clean designs with clear separation of concerns so that we can refactor our code as necessary to meet the changing demands of the customer; we use proven patterns and practices so we can stay out of dangerous waters; we use nifty tools to help increase our productivity. But the thing that doesn’t get talked about enough is the inflexible customer.

There’s a phrase where I work that crops up in just about every conversation we have with our customer: We’ve always done it that way. That phrase has has single-handedly added more lines of code and more complexity to this project that anything else I can think of. It has destroyed our ability to simplify our design. It has lengthened our development time considerably.

And it has ripped out my heart.

I love software development – when it is software development. What I don’t enjoy is having the customer rigidly dictate the design of an application based on some pre-existing paper process and then making sure to squash every effort to improve the workflow or the software by repeating the mantra, “But we’ve always done it that way.”

I can honestly say nothing kills my excitement – nothing quenches my fire – faster than, “We’ve always done it that way.” I imagine it is like staring at a hideous mess of code and having someone say, “You are not allowed to touch that – don’t refactor that at all – we’ve always written code that way.”

I’m curious to know what other developers do to survive a customer like this. What keeps your passion afloat when you’re presented with this situation? I’m having to fall back on Personal Pride and Work Ethic. And honestly, I hate those guys. Yeah, they get the job done, but they’re so much less pleasant to be around than, say, Passion and Excitement.

One of the tenets of the Agile Manifesto reads:

At regular intervals, the team reflects on how to become more effective, then tunes and adjusts its behavior accordingly.

The past few days have been one of those opportunities. In part because of the way our workload was structured, but also because of pain.

For a long time we’ve been using stubs to perform unit testing. This works, but comes with a certain level of baggage. That baggage can turn into pain; pain that can grow over time as the project increases in size. Our project has grown sufficiently large enough that the pain has become more acute, and so we started looking at alternatives.

Through Google, I ran across this post from Alex McMahon. Alex was also using stubs, and wondered if a mock framework would be better. He enumerates the benefits of a mocking framework quite well in his post. The point that he made that resonated the most with me was this:

Decoupling of tests – by not using a single hand coded mock we will avoid coupling tests together by sharing a single mock.

When we started unit testing, this was the road we went down as well: hand-coded stubs. We created stub classes with preloaded data for testing, and then we used those across the suite of unit tests. You can probably imagine the pain that sort of coupling eventually caused.

Using stubs for unit testing equates to state-based unit testing. A typical unit test will involve doing something like fetching a stubbed business object or DTO from a stubbed service, manipulating the stubbed object (like adding an item to a list), and then fetching that stubbed object off a stub view, and asserting a value (for instance, a count).

Problems occur whenever the data for a stub class changes; maybe because of some new requirement for a new test case, or because someone needed additional data for a particular test. Every time the stub data changes it can affect other unit tests and test fixtures. You can try and mitigate that by creating different subsets of the same stub classes with different suites of data, but that becomes a lot of work very quickly.

Coupling unit tests to a hard-coded stub is a bad thing. It might not look like it early on, but as your application grows and requirements change, the problem becomes compounded. This coupling makes tests brittle. Brittle tests are the tools of the devil – they erode developer confidence, which is exactly the opposite of what we’re trying to achieve with unit testing.

With that in mind, I decided to take a look at RhinoMocks. I read Oren’s blog every day through my aggregator. His blog is a great window into the mind of a great developer, so I knew RhinoMocks would rock. It’s also free. And from what I heard while at the ALT.NET conference, RhinoMocks was pretty widely adopted (as was TypeMock).

The first thing you have to get past when switching from stubs to a mocking framework is the syntax. It’s just different. But very quickly it starts to read clearly – much more clearly than a stubbed test fixture. Tests become easier to understand and you can figure out the meaning of a unit test much faster when you can read something like this:

Expect.Call(service.GetEmployee(6)).IgnoreArguments().Return(employee);

Other than the new syntax, the biggest change is switching from state-based testing to interaction-based testing. And I have to say, after some time with RhinoMocks I enjoy the interaction-based testing much better.

Take, for instance, the following test. We want to ensure that a service call is made and a list of Department names is fetched and then passed to the view for binding to a drop down list. To achieve this with stubs, we have to have concrete stubs for the Service & View, and we have to assert the object off the View to make sure it reached its destination. More complicated logic might also test the count of a list, or specific data values on the object that the view contains.

[Test]
public void OnViewReady_FetchesDepartmentNamesFromService_AndUpdatesView()
{
   IPayrollService service = new Stubs.PayrollService();
   IDepartmentsView view = new Stubs.DepartmentsView();
   List departmentNames = new List();
   service.DepartmentNamesDTOReturnValue = departmentNames;

   DepartmentsViewPresenter presenter = new DepartmentsViewPresenter(service);
   presenter.View = view;

   presenter.OnViewReady();

   Assert.AreEqual(departmentNames, view.DepartmentNames);
}

This ends up being a very simple test. But as you begin to test more complicated logic, your asserts quickly grow in number and complexity. Frequent assertions include testing to make sure list counts meet some expected value, or specific fields have certain values.

Imagine testing a list’s Count property to ensure a new item created by the user was added to the list. Now imagine a few days later a fellow developer adds a new item to the default stubbed list. All of your unit tests will fail now because the count will be off by one in the stubbed list.

These problems happen because we’re using state-based testing to verify that a method call or an event publication/subscription actually occurred. It is a roundabout way to perform unit testing on classes that don’t need to worry about state. And this is where interaction-based testing shines.

The same test in RhinoMocks looks something like this:

[Test]
public void OnViewReady_FetchesDepartmentNamesFromService_AndUpdatesView()
{
   MockRepository mocks = new MockRepository();
   IPayrollService service = (IPayrollService)mocks.CreateMock(typeof(IPayrollService));
   IDepartmentsView view = (IDepartmentsView)mocks.CreateMock(typeof(IPayrollService));
   List departmentNames = new List();

   DepartmentsViewPresenter presenter = new DepartmentsViewPresenter(service);
   presenter.View = view;

   
   Expect.Call(service.GetAllDepartmentNames()).Return(departmentNames);
   view.DepartmentNames = departmentNames;
   LastCall.On(view);
   

   presenter.OnViewReady();

   mocks.VerifyAll();
}

What’s great about the RhinoMocks version is that it reads like how you think the program logic should execute. I expect GetAllDepartmentNames to be called with no parameters. I expect the list of DepartmentNameDTO’s to be returned when that call is made. I expect the DepartmentNames property to be set on the view. I don’t care what the list of DTO’s looks like, how much data it has, what the list count is or anything else. Only that methods get called and properties get set.

My test is not dependent on any external classes – everything is mocked for my by RhinoMocks. The test is self-contained. Changes to other tests in this test fixture – or any other test fixture for that matter – won’t cause this test to break. It’s not brittle anymore.

The benefits to this become apparent when you start using the mock framework for more complicated tests, because you can eliminate all data that is state-based and just worry about the flow of the logic. Did the right methods get called? In the right order? The right number of times? Did an event get wired up correctly? Did an event get fired? Caught? Did methods that are not supposed to be called get called? Do I even care?

RhinoMocks lets you ask these questions and answer them in a way that is direct and intuitive.

And, of course, for classes where you do still need to do state-based testing, RhinoMocks lets you do that as well.

Overall, I’m extremely pleased we’ve started using RhinoMocks. I’m more pleased that we’ve moved on from state-based unit testing in classes where it’s not necessary. In short: I’m sold.

Here’s an interesting post courtesy of Moorgard’s blog: Scott Hartsman spent some time to respond to criticism about the relative quality and complexity of MMORPG’s with a blog post titled: MMOs are bigger than you think. Scott is a guy who has worked on Everquest and Everquest II, so he should know what he’s talking about.

The reason I got into software development was because of video games. My first real career goal as a software developer – the first “dream job” I ever really contemplated – was to work on a great video game. So it was with a certain amount of anger and frustration that I watched as games that I loved to play, and companies who made those games, suffered criticism at the hands of ignorant players.

I’ve long defended the developers of Everquest and Everquest II on various message boards, largely because I, being a software developer, understood the complexities and hurdles that those developers face to bring us these games that we enjoy so much. So it was with great interest that I read Scott’s post today.

While reading his post it struck me how similar his problems are to any other business application. Scott writes:

This is painful for MMOs in particular because of the unique (huge) number of critical, non-sexy things that you have to succeed at, where failing at any one of them can entirely sink your game:

- Pipelines
- Tools
- Infrastructure
- Stability (again, doubling the work – the client and all the servers)
- Scalability
- Stability
- Security (added this in for the blog post – Can’t trust that client)
- Performance (optimize both that client and all those server processes)
- Oh, and..Stability

Are these things really any different from an ERP system, for instance? Looking down the list, I see all the same issues that we (.NET business application developers) face in our domain as well.

Which makes me wonder if part of the problem isn’t the methodology. Scott brings up an interesting point:

MMOs are still really young. To a lot of the people working on them, it very much is creating something entirely new. Compare to movies or single player games, for instance. It’s less of a challenge to staff those types of projects up with people who’ve worked on them before, in all of the right positions. Doing the same on a high-budget MMO remains next to impossible.

I don’t mean “key management” or “leads” like you see in studio announcements and press releases all the time. I mean everyone other than a small number of entry-level folks. Until you’ve done it once, you have no idea what you’re getting yourself into.

But you know what? This is true for all software development, not just MMOG’s.

Just about every job I’ve had, I’ve been asked to develop software that I’ve never done before. Of course, that’s part of what makes this industry so damn cool – we’re always learning new stuff, and we’re always pushing our limits with new technologies, tools and ideas. But the core problem is the same: we work in an industry where we’re asked to do things that we have never done before.

I don’t think this is anything unique to MMOG’s. Sure, there are people in the business application world who have built business software a dozen times. But for every guy who finally achieves a level of expertise with that sort of domain, a dozen other guys graduate from college without any experience.

So I come back to the methodology. In the business software world, we’re learning that Waterfall is a recipe for failure, and Agile is a way to help us succeed. Methodologies like Scrum and Extreme Programming are giving software developers better techniques for minimizing risk and ensuring success. I’m starting to read more and more where game development houses are experimenting with or turning to Agile to help them succeed as well. I think these things add up…

Inexperienced developers is an issue that will never go away; it is going to remain very hard in the future to hire only the people with extensive knowledge in a particular domain. As more new blood enters the workforce, our discipline will continue to be refreshed with talented developers who lack in specific domain expertise. Mentors will always be key, and so will methodologies that help teams succeed.

The trick, as I see it, is to improve the process of software development. When you can accomplish that, even in small measures, then some of the issues Scott raises, like inexperienced developers and “wild miss-scoping” can be minimized.