Tuesday, November 10, 2009

Threads Updating 64-bit Variables

I've been reading "Java Concurrency in Practice" by Brian Goetz lately, and he described something that I've heard about, but never really been a witness to. In the chapter 3, when describing synchronisation, and atomic operations there is a mention of 64-bit variables. He describes:

"The Java Memory Model requires fetch and store operations to be atomic, but for nonvolatile long and double variables, the JVM is permitted to treat a 64-bit read or write as two separate 32-bit operations. If the reads and writes occur in different threads, it is therefore possible to read a nonvolatile long and get back the high 32-bitsof one value and the low 32 bits of another."

Sounds fair, but I have never really seen this in the flesh. So, I threw together a little program to demonstrate this behaviour.

package com.codewax.threadtests.sixtyfourbit;

import java.util.HashSet;
import java.util.Random;
import java.util.Set;
import java.util.concurrent.*;

public class Example {

public static void main(String[] args) throws Exception {
displayReport(new Example().runFor(100000));
}

private static void displayReport(Set<Long> results) {
displaySystemProperties("java.version", "java.runtime.version",
"java.vm.version", "java.vm.name");
System.out.println("\nResults");
System.out.println("-------");
System.out.println(results.size() + " alien numbers found");
}

private static void displaySystemProperties(String... propertyNames) {
System.out.println("System Configuration");
System.out.println("--------------------");
for (String propertyName : propertyNames) {
System.out.println(propertyName + "=" +
System.getProperties().getProperty(propertyName));
}
}

private static final long STARTING_NUMBER = 0;
private long source = STARTING_NUMBER;

private Set<Long> runFor(int numberOfOperations) throws Exception {
ExecutorService service = Executors.newCachedThreadPool();
Future<Set<Long>> numbersWritten =
service.submit(writing(numberOfOperations));
Future<Set<Long>> numbersRead =
service.submit(reading(numberOfOperations));
service.shutdown();
service.awaitTermination(10, TimeUnit.SECONDS); //plenty!
return findAlienNumbers(numbersRead.get(), numbersWritten.get());
}

private Set<Long> findAlienNumbers(Set<Long> numbersRead,
Set<Long> numbersWritten) {
numbersRead.remove(STARTING_NUMBER);
numbersRead.removeAll(numbersWritten);
return numbersRead;
}

private Callable<Set<Long>> reading(final int totalOperations) {
return new Callable<Set<Long>>() {
public Set<Long> call() throws Exception {
return new HashSet<Long>() {{
int operationsSoFar = 0;
while (operationsSoFar++ < totalOperations) {
add(source);
}
}};
}
};
}

private Callable<Set<Long>> writing(final int totalOperations) {
return new Callable<Set<Long>>() {
public Set<Long> call() throws Exception {
final Random random = new Random();
return new HashSet<Long>() {{
int operationsSoFar = 0;
while (operationsSoFar++ < totalOperations) {
long next = random.nextLong();
source = next;
add(next);
}
}};
}
};
}

}


This example should be fairly self-explanatory, but for those of you who would like a description, read on.

The main parts of this class are
  1. The writer

  2. The reader

  3. The alien number finder.

(hmm, sounds like a poem).

The writer (1) writes 100000 random numbers to a source 64-bit variable, recording in a set which numbers it wrote along the way.

The reader (2) reads 100000 times from the source variable.

The alien number finder (3) basically removes from the list of read numbers any numbers that were written (and the starting number).

There are a few caveats to this code. One of which being that when the callable's are submitted to the thread pool, they start immediately. The writer thread has some time to write a number of values before the reader thread has time to start reading. On average, by the time the writer thread has completed and written 100000 values, the reader thread will have only read around 4000 values.

Still, I feel that even with these numbers, this represents enough data to see this behaviour.

On to running the code!

First attempt...
agentdh-2:ThreadTests rbarlow$ java -cp out/production/ThreadTests \
com.codewax.threadtests.sixtyfourbit.Example
System Configuration
--------------------
java.version=1.6.0_15
java.runtime.version=1.6.0_15-b03-219
java.vm.version=14.1-b02-90
java.vm.name=Java HotSpot(TM) 64-Bit Server VM

Results
-------
0 alien numbers found


What has happened here? At first glance, it seems that the code must be incorrect-0 alien numbers fonud (alien being numbers that were not written, but were read). But this is not correct. The problem is not with the code, but with the platform! I'm running a MacBook with Snow Leopard installed. Snow Leopard defaults to running a 64-bit version of Java 1.6 (as can be seen by the report output before the results).

Lets swap over to a 32-bit java version, and try again.

Second attempt...
System Configuration
--------------------
java.version=1.6.0_15
java.runtime.version=1.6.0_15-b03-219
java.vm.version=14.1-b02-90
java.vm.name=Java HotSpot(TM) Client VM

Results
-------
319 alien numbers found


Yes! There it is. 319 alien numbers were read during the operation.

This obviously shows that the way that the JVM reads and writes 64 bit values is dependant on the implementation. In the earlier case, the registers used were 64 bit, so there is no need to read or write values in more than one operation.

Of course, none of this would be a problem if you marked the variable as volatile, or used proper synchronisation!

Monday, October 19, 2009

Using Stub Repositories for Fast Testing

In this post, I would like to describe a technique for testing, and at times debugging, that I have recently employed. I have found it very useful for fast feedback and would like to share my experiences.

Recently I was looking for a better, faster way to run my tests. Some of the problems I had to overcome were:
  1. Slow Testing
    I have a large suite of user acceptance tests. Many of these tests define setup criteria (which effectively pre-populates a database), execute the test modifying the system in some manner, and tears down the test including the a database reset (delete all the data from the databases). As you know, and have most likely experienced, this can be frustratingly slow-especially on large volumes of data.
  2. Difficult Debugging
    When breaking into an execution of a test, I would like to see the state of my system. If my repositories have persisted their data to a database, it can be difficult for me to get at that data-especially in the middle of an uncommitted transaction.
So what I needed was to improve the overall speed of my build, and to be able to take a snapshot of my repositories at any point in time. I achieved this using Stub Repositories.

In the book 'Domain Driven Design' by Eric Evans, repositories are:

"A repository represents all objects of a certain types as a conceptual set (usually emulated). It acts like a collection, except with more elaborate querying capability. Objects of the appropriate type are added and removed, and the machinery behind the repository inserts them or deletes them from the database."

In practice, I use a repository to find existing objects, and also to persist newly created ones. Without getting into too much detail, it is used as the layer between my application and the database. The behaviour of a repository is to query the database for an object that meets the criteria of my input, and restore the saved state of an object with the data that is has found. I use repositories to manage instances of in memory objects.

A stub is, according to Martin Fowler:

"Stubs provide canned answers to calls made during the test, usually not responding at all to anything outside what's programmed in for the test. Stubs may also record information about calls, such as an email gateway stub that remembers the messages it 'sent', or maybe only how many messages it 'sent'."

By combining these 2 concepts, I get what I've referred to as a StubRepository (can you see what I did there?).

For any repository implementation in my application, I define an interface describing all the query and collection management actions that can be performed on a particular type of entity. Once I have this interface defined, I can then implement both a real version of it that uses a real database, and a stub version that is purely in memory.

As an example, let's define an Employee Repository interface.

public interface EmployeeRepository {
Employee findBy(EmployeeNumber number);
List<Employee> findBy(EmployerName employer);
void add(Employee employee);
void delete(Employee employee);
}
A typical HibernateEmployeeRepository (which of course, implements EmployeeRepository) could look like this:

public class HibernateEmployeeRepository extends HibernateRepository
implements EmployeeRepository {
public HibernateEmployeeRepository(SessionProvider sessionFactory) {
super(Employee.class, sessionFactory);
}

public Employee findBy(EmployeeNumber number) {
return findUnique(eq("number", number));
}

public List<Employee> findBy(EmployerName employer) {
return find(eq("employer", employer));
}

public void add(Employee employee) {
getCurrentSession().save(employee);
}

public void delete(Employee employee) {
getCurrentSession().delete(employee);
}
}
So long as my application only ever uses the interface (beyond repository creation), my application can query, add and delete employee objects, without having to know what database magic is happening under the hood. And because I have this well defined interface, I can now create a stub version of it for use in testing. I generally do this using maps and collections. Because a repository has collection semantics, a collection should be all that is required to implement the stub. If not, this may be a code smell.

So lets implement a stub version of the employee interface.

public class StubEmployeeRepository extends HibernateRepository
implements EmployeeRepository {
private List<Employee> employees = new ArrayList<Employee>();
private Map<EmployeeNumber, Employee> byNumber =
new HashMap<EmployeeNumber, Employee>();
private Map<Employer, Employee> byEmployer = new HashMap<Employer, Employee>();

public Employee findBy(EmployeeNumber number) {
return byNumber.get(number);
}

public List<Employee> findBy(Employer employer) {
return byEmployer.get(employer);
}

public void add(Employee employee) {
employees.add(employee);
byNumber.put(employee.getNumber(), employee);
byEmployer.put(employee.getEmployer(), employee);
}

public void delete(Employee employee) {
employees.remove(employee);
byNumber.remove(employee.getNumber());
byEmployer.remove(employee.getEmployer());
}
}
So now I have the following:This now lets us use either implementation of the repository at runtime. Of course, only the hibernate version will be used in production code, but for our tests, I have the luxury of swapping in the stub, rather than using the real hibernate repository. Have I overcome the earlier problems?
  1. Speed? Yes!In memory maps and collections are fast.
    For example, in some of our acceptance criteria, we are required to create and manage over 30 (average sized) objects. Using the hibernate repositories (and Oracle) takes just over 10 seconds. Using stubs takes around 0.5 seconds. This is 20 times faster! And if you extrapolate this to all of your acceptance tests, it is easy to see the benefit.
  2. Easy debug? Yes!When running acceptance tests, quite often it is nice to stop and have a look at the state of your system part way through. With the hibernate repository and newly created objects would have been saved to the database. Assuming that these are being saved from inside a transaction, it can be difficult to query the database to see the current state. When using the stub repository, you can easily look at the contents of a map or collection.
You can get close to both of these by using an in memory database. Hibernate is very nice and will create an in memory database for you for testing, auto generated from the hbm's. But the setup and teardown time of this database a lot slower than stubs. Setup and teardown of the stub repositories is extremely fast. And again, maps and collections are much easier to debug than an in memory database.

This is quite cool, but there are always disadvantages to any idea. One disadvantage I found was that whenever I added a new method to the repository interface, I needed to implement it in both the hibernate and stub versions. Here lie dragons. What if my implementations diverged? For example, what if the list returned by the repository version was sorted differently from my stub version. I could potentially be causing myself future headaches.

To get around this, I use the tests that I wrote for the hibernate version of the repository to test both repositories. This is done by moving the tests into an abstract repository test (see http://c2.com/cgi/wiki$?AbstractTestCases). I then extend this abstract to create a test for the hibernate and the stub repositories as such:As you can see from this diagram, each of the concrete tests create their own implementation of a repository in the createRepository() method. The HibernateEmployeeRepositoryTest creates a HibernateEmployeeRepository and the StubEmployeeRepositoryTest creates the StubEmployeeRepository. These are used in the abstract test.

Any future (test driven) functionality that is added to my repository is tested at the abstract test level. This way I can be certain that both the hibernate and stub versions of the repository include the functionality and behave the same. For example, if I decided that the repository should throw an EmployeeNotFound exception in the findBy methods, when testing for it, it would require that this new behaviour be exhibited by both implementations.

Now you may be asking, how do you switch the implementations? I use an environment factory create the repositories for me, and a switch to give me either a real (hibernate) factory, or a stub factory.

For example:

public interface ApplicationTestingEnvironment {
EmployeeRepository createEmployeeRepository();
PayrollRepository createPayrollRepository();
}

public class HibernateTestingEnvironment implements ApplicationTestingEnvironment {
private final SessionFactory sessionFactory;

public HibernateTestingEnvironment(SessionFactory sessionFactory) {
this.sessionFactory = sessionFactory;
}

public EmployeeRepository createEmployeeRepository() {
return new HibernateEmployeeRepository(sessionFactory);
}

public PayrollRepository createPayrollRepository() {
return new HibernatePayrollRepository(sessionFactory);
}
}

public class StubTestingEnvironment implements ApplicationTestingEnvironment {
public EmployeeRepository createEmployeeRepository() {
return new StubEmployeeRepository();
}

public PayrollRepository createPayrollRepository() {
return new StubPayrollRepository();
}
}

public enum ApplicationTestingFactory {
USE_STUBS{
return new StubTestingEnvironment();
},
USE_REAL{
return new HibernateTestingEnvironment(new SessionFactory());
},
CHOOSE_USING_SYSTEM_PROPERTY() {
if ("false".equals(System.getProperty("use.stubs"))) {
return USE_REAL.create();
}
return USE_STUBS.create();
}
public abstract ApplicationTestingFactory create();
}
One of the nice things I've done here is to include the system property as a decider for using a stub or a hibernate repository. This allows us to kick off testing from the command line using either stubs or real. Mainly because of the fast feedback, I've defaulted to using the stub repositories. But on the continuous integration server, you can leave it using real databases. This can be done as follows:
ant full-build -Duse.stubs=false
I hope this helps. If you have any questions of comments, please don't hesitate to add a comment below.

Sunday, April 5, 2009

Unknown error when syncing iPod (-50)--solved!

For about a week now, I've been getting a unknown -50 error every time I tried to sync my iPod (strange that an unknown error actually has an error code!). After much scouring of the internet, I discovered that it was something to do with my iPhoto library.

I went to the iTunes settings for my iTouch, selected the Photos tab and unselected the 'Sync photos from' option-thus erasing all photos from my iTouch. After this I was able to successfully sync my iTouch-yay! back in action. If I tried to enable syncing photos to the iTouch, it would -50 error at me again. Definitely a problem with syncing photos.

Now although this fixed the immediate problem, I was still a little unhappy. I quite regularly use my iTouch to show friends and colleagues images of my recent jaunts. So I needed to track down the actual problem.

I tried using OSX's console application to discover what was happening-but no errors were being reported here.

I decided to then have a dig around the files in the iPhoto library's directory. It was here that I found my culprit. I tried to open AlbumnData.xml in Safari and it turns out that the XML file was not well formed. About a week ago I had renamed my iPhoto library to one that included both a quote character and an ampersand. iPhoto had quite happily written that library name directly into the XML document without Unicoding the special characters.

I renamed my library (i.e. the iPhoto directory) and manually corrected the name in AlbumData.xml. Now I can sync my photos again and bore my friends and colleagues no end.

Summary steps to fix the -50 error:
1. Open iTunes and deselect the syncing of photos for your device
2. Using Safari or Firefox, open the file AlbumData.xml that is contained in your iPhoto library directory. You should get an error here describing where in the xml document it is malformed.
3. Using a text editor (I used vi), open AlbumData.xml and correct the malformed XML
4. If you renamed the library in the XML in the previous step, change the directory name of the library to the same name
5. Whilst holding down the ALT key, click on iPhoto to start it. This should give you a dialog where you can select to choose which iPhoto library to Open. Click 'Choose Library...' and select your newly renamed library.
6. In iTunes, go to the Photos tab for your device, and select sync photos from iPhoto
7. Apply and Resync

Job's a good-un!!

Monday, March 23, 2009

Expressing time as a domain object, instead of insane long calculations


Hands up everyone here who has had to represent time as a long? OK, next question, how many of you have had to express a large amount of time (for example 7 days) in milliseconds? I bet you've done this:
7*24*60*60*1000

Kind of nice, and I bet you've even refactored like this:
HOURS_IN_A_DAY*MINUTES_IN_AN_HOUR*SECONDS_IN_A_MINUTE*MILLISECONDS_IN_A_SECOND

OK, getting better. It's a bit clearer as to what all those troublesome little numbers mean-but something still not quite right. What if, say for example, you pass this result into a method that accepts time in a different unit of measure? eg.
void sleepFor(long seconds) {
Thread.sleep(seconds*1000);
}

Now the compiler isn't going to complain, you're going to be waiting a lot longer than you anticipated-say 1000 times longer? We've passed the wrong unit of time into this method. Our constant is represented in milliseconds, yet this method accepts time as seconds, but they are both the same datatype! But, I hear you say, we don't want a different datatype for every unit of time-that would be madness. Indeed it would.

To stop the world descending into a pit of chaos, overlorded by mad monkeys*, try this. Don't represent time as a long, represent the amount of time you are interested in as a long, and combine it with a TimeUnit!

Call it what you like, but I called it Duration. An example of which can be found in a project I'm currently working on called gibble here. This is only a small example of the Duration class that I use elsewhere.

Now, we can represent the above code as follows:
sleepFor(days(7)); //static import of Duration here

And the sleepFor method like this:
void sleepFor(Duration timeout) {
Thread.sleep(timeout.inMillis());
}

What's so cool is that we can never be tricked into sleeping for the wrong amount of time! I hate being tricked into sleeping 1000 times more that I have too. Sleep too much and the monkeys might take over!

* Would that be any worse/different that what we have now?

Tuesday, March 10, 2009

Micro DSL meets testing

Last night I was writing a number of tests. All the tests were quite similar, so there was potential for much duplication. An example of one of the tests was this:
@Test
public void exampleTest() {
context.checking(new Expectations(){{
one(serviceA).getValue(); will(returnValue(SMALL_NUMBER));
one(serviceB).getValue(); will(returnValue(BIG_NUMBER));
}});
assertThat(new ValueChooser().chooseFrom(serviceA.getValue(),
serviceB.getValue()),
is(equalTo(SMALL_NUMBER);
}

I basically needed to assert the result of the value chooser, based on what the 2 services returned. So, to avoid duplication, instinct tells me to extract method, and parameterise variable parts. This would give you a method like so:
assertValueChosen(serviceAResponse, serviceBResponse, expectedValue);

When looking at this, it is not too difficult to tell what the parameters are, but if you actually place values into the parameter list, it can detract from the readability of the assert. For example:
assertValueChosen(SMALL_NUMBER, BIG_NUMBER, SMALL_NUMBER);

The name of the test could describe what is being asserted, and yes, this should definitely be correct because this is what is output in the JUnit results (and thus reported on), but it is quite easy (like it is with comments) to change the code, and have the name of the tests method no longer describe exactly what it is that test is testing. The name of the test could be out of date.

I had a small think about it, and decided that, what I would like would be a sentence that read like an example of behaviour. "When service A returns x, and service B returns y, then assert the value chosen is z" kind of thing.

What I decided on was to use a Micro DSL. Doing so allowed me to express my tests more clearly and not have to rely on the name of the test:
whenServiceAReturns(SMALL_NUMBER).andServiceBReturns(BIG_NUMBER).
assertValueChosenIs(SMALL_NUMBER);

Lovely! It is now impossible for this sentence to be out of date, from what is being executed.

So now, for your viewing pleasure, here is the whole test in its entirety...
public class ValueChooserTest {

private static final int SMALL_NUMBER = 5;
private static final int BIG_NUMBER = 10;

@Test
public void testAllCases() {
whenServiceAReturns(SMALL_NUMBER).andServiceBReturns(BIG_NUMBER).assertValueChosenIs(SMALL_NUMBER);
whenServiceAReturns(BIG_NUMBER).andServiceBReturns(SMALL_NUMBER).assertValueChosenIs(SMALL_NUMBER);
whenServiceAReturns(null).andServiceBReturns(SMALL_NUMBER).assertValueChosenIs(SMALL_NUMBER);
whenServiceAReturns(SMALL_NUMBER).andServiceBReturns(null).assertValueChosenIs(SMALL_NUMBER);
whenServiceAReturns(null).andServiceBReturns(null).assertValueChosenIs(0);
}

private class ValueChooserAsserter {
private Integer serviceAValue;
private Integer serviceBValue;

private final Mockery context = new JUnit4Mockery();
private final Service serviceA = context.mock(Service.class, "serviceA");
private final Service serviceB = context.mock(Service.class, "serviceB");

private ValueChooserAsserter(Integer serviceAValue) {
this.serviceAValue = serviceAValue;
}

public static ValueChooserAsserter whenServiceAReturns(Integer value) {
return new ValueChooserAsserter(value);
}

public ValueChooserAsserter andServiceBReturns(Integer serviceBValue) {
this.serviceBValue = serviceBValue;
return this;
}

public void assertValueChosenIs(Integer expectedValue) {
context.checking(new Expectations(){{
one(serviceA).getValue(); will(returnValue(serviceAValue));
one(serviceB).getValue(); will(returnValue(serviceBValue));
}});
assertThat(new ValueChooser().chooseFrom(serviceA.getValue(), serviceB.getValue()), is(equalTo(expectedValue)));
}
}

}

So people, what do you think? :)

---------- UPDATE ----------

Here is the example modified after Romilly's comments:

public class ValueChooserTest {

private static final int SMALL_NUMBER = 5;
private static final int BIG_NUMBER = 10;

@Test
public void testAllCases() {
whenServiceAReturns(SMALL_NUMBER).andServiceBReturns(BIG_NUMBER).assertValueChosenIs(SMALL_NUMBER);
whenServiceAReturns(BIG_NUMBER).andServiceBReturns(SMALL_NUMBER).assertValueChosenIs(SMALL_NUMBER);
whenServiceAReturns(null).andServiceBReturns(SMALL_NUMBER).assertValueChosenIs(SMALL_NUMBER);
whenServiceAReturns(SMALL_NUMBER).andServiceBReturns(null).assertValueChosenIs(SMALL_NUMBER);
whenServiceAReturns(null).andServiceBReturns(null).assertValueChosenIs(0);
}

private class ValueChooserAsserter {
private Integer serviceAValue;
private Integer serviceBValue;

private final Mockery context = new JUnit4Mockery();
private final Service serviceA = context.mock(Service.class, "serviceA");
private final Service serviceB = context.mock(Service.class, "serviceB");

private ValueChooserAsserter(Integer serviceAValue) {
this.serviceAValue = serviceAValue;
}

public static ValueChooserAsserter whenServiceAReturns(Integer value) {
return new ValueChooserAsserter(value);
}

public ValueChooserAsserter andServiceBReturns(Integer serviceBValue) {
this.serviceBValue = serviceBValue;
return this;
}

public void assertValueChosenIs(Integer expectedValue) {
context.checking(new Expectations(){{
one(serviceA).getValue(); will(returnValue(serviceAValue));
one(serviceB).getValue(); will(returnValue(serviceBValue));
}});
assertThat(new ValueChooser(serviceA, serviceB).choose(), is(equalTo(expectedValue)));
}
}

}

Monday, February 23, 2009

Micro DSL list finder versus Jedi

Toby "blog-a-lot" Weston wrote about a micro DSL style list finder that he is playing around with. His list finder looks like this:

Employee candidate = find(interviewee).in(applicants).using(superStrictCriteria);


where the list finder uses a custom comparator interface (in this case the superStrictCriteria) to determine if a target (interviewee) matches any of the list elements (applicants). I think this DSL is quite nice-but there is another library that does this for you already in a similar fashion-Jedi. Jedi is a functional programming library that allows you to express your ideas in a very computer-sciencey way. Impress your collegues and try it today!

The above example, using Jedi can be done as follows:

Employee candidate = first(select(applicants, superStrictCriteria));


where superStrictCriteria is a Filter interface which essentially boils down to:

public interface Filter <T> {
boolean execute(T value);
}


so in this case could be:

new Filter<employee>() {
public Boolean execute(Employee employee) {
return superStrictCriteria.canBeAnsweredBy(employee);
}
}

(Note: in the actual Jedi code the Filter interface extends Functor-but I left that bit out for brevity)

An advantage that you get with Jedi is that there are already a lot of functional ways to handle lists. Have a look in the jedi.functional.FunctionalPrimitives class. You can mix and match pretty much any of these functional primitives to attain quite complex list handling and criteria selection. And if this is not enough, Jedi gives you Coercions (eg. converting lists and arrays), Comparables (eg. equal, lessThan, min), FirstOrderLogic (eg. exists, all, not) and much much more!

So, although I'm not one for reinventing the wheel (I would normally use Jedi for what Toby describes), I do like to explore new styles of coding-just so long as the substance is there.

Tuesday, February 10, 2009

Substance Over Style

I find that too much effort and emphasis is put on coding style. We blindly follow the guidelines and styles that we've always done, without questioning why.

In an Agile process, one of the goals that we strive for is continual improvement, continual quality process review. With every line of code that we write, we should do the same. Question why we are writing that code, or why we are writing it that way.

A good example of this is the if statement in Java. One of the commonly followed guidelines is that you should alway enclose the block of an if statement in brackets-even if it only one line of code. For example:

if(toast.isBurnt()){
eat(somethingElse);
}


instead of:

if(toast.isBurnt()) eat(somethingElse);


A traditional argument for this is that a developer may come along and add more code into the if block, but forget to add the brackets. Thus following a coding style convention will avoid these problems.

What is wrong with this statement? In a proper Agile environment, where tests are written first, surely this 'problem' will be caught the very next time you run your unit tests? You know, testing the substance of your code?

For my money, I'd prefer the developer to be writing test first code. As long as the substance can be proven, I'm not too bothered with the style of the code (within reason, of course!).

So, put your effort into the substance, and worry about the style later.