Apache VFS is a great way to access different file systems in the same way. I particularly like the custom RAM filesystem in my unit tests in order to check code that eventually accesses the actual file system.

For example:

StandardFileSystemManager manager = new StandardFileSystemManager();
manager.init();
manager.resolveFile("ram://root/file.txt").createFile(); // prepare test harness
...
manager.resolveFile("ram://root/file.txt").getName(); // do something useful

Annoyingly, Apache VFS API provides a way to check if a file/folder is hidden, but, although it is always possible to manually create hidden files on your system, there is no way to mark a file as hidden in the RAM filesystem.

manager.resolveFile("ram://root/file.txt").isHidden(); // returns false
manager.resolveFile("ram://root/.file.txt").isHidden(); // also returns false

The solution is to modify the RAM filesystem and customize its checking of the hidden attribute. This means that we need a new RamFileProvider, a new RamFileSystem and finally a new RamFileObject. In this example, all files and folders whose names start with a dot will be considered hidden:

public class RamFileProvider extends org.apache.commons.vfs2.provider.ram.RamFileProvider {
  @Override
  protected FileSystem doCreateFileSystem(FileName name, FileSystemOptions fileSystemOptions) throws FileSystemException {
    return new RamFileSystem(name, fileSystemOptions);
  }

  private static class RamFileSystem extends org.apache.commons.vfs2.provider.ram.RamFileSystem {
    public RamFileSystem(FileName rootName, FileSystemOptions fileSystemOptions) {
      super(rootName, fileSystemOptions);
    }

    @Override
    protected FileObject createFile(AbstractFileName name) throws Exception {
      return new RamFileObject(name, this);
    }
  }

  private static class RamFileObject extends org.apache.commons.vfs2.provider.ram.RamFileObject {
    public RamFileObject(AbstractFileName name, RamFileSystem ramFileSystemWithHiddenSupport) {
      super(name, ramFileSystemWithHiddenSupport);
    }

    @Override
    public boolean isHidden() throws FileSystemException {
      // this is the important part
      if (getName().getBaseName().startsWith(".")) {
        return true;
      }
      return super.isHidden(); // in practice, this always return false
    }
  }
}

Finally, this new RamFileProvider must be registered with your existing FileSystemManager. In my case, I get something like that:

StandardFileSystemManager manager = new StandardFileSystemManager();
manager.init();
manager.addProvider("ram-ext", new RamFileProvider());
manager.resolveFile("ram-ext:///root/file.txt").createFile();
manager.resolveFile("ram-ext:///root/.file.txt").createFile();
...
manager.resolveFile("ram-ext://root/file.txt").isHidden(); // returns false
manager.resolveFile("ram-ext://root/.file.txt").isHidden(); // returns true

Note that I did not replace the “ram” filesystem entirely. Although that would have been my preferred option, that would have taken too much time in my little experiment.

My former colleague David just posted an example of verbose test code on his blog (the parts in French have been translated to English by myself):

/**
 *
 */
@Test
public void testGetCustomerOK() {

  LOGGER.info("======= testGetCustomerOK starting...");

  try {
    CustomerDTO targetDTO = this.serviceImpl.getCustomer("ABC99");

    // Check.
    Assert.assertNotNull("Extra not found.", targetDTO);
    Assert.assertEquals("accountIds must be the same.", "ABC99", targetDTO.getAccountId());

  } catch (CustomerBusinessException exception) {
    LOGGER.error("CustomerBusinessException : {}",
	    exception.getCause());
    Assert.fail(exception.getMessage());
  } catch (UnavailableResourceException exception) {
    LOGGER.error("UnavailableResourceException : {}",
	    exception.getMessage());
    Assert.fail(exception.getMessage());
  } catch (UnexpectedException exception) {
    LOGGER.error("UnexpectedException : {}" +
	    exception.getMessage());
    Assert.fail(exception.getMessage());
  } catch (Exception exception) {
    LOGGER.error("CRASH : " + exception.getMessage());
    Assert.fail(exception.getMessage());
  }

  LOGGER.info("======= testGetCustomerOK done.");
}

Facing this code, the first thing I would do is remove the logs in the catch clauses, which are clearly obscuring the code the most. They do not provide actual value, since the exception message is used in Assert.fail() calls.

/**
 *
 */
@Test
public void testGetCustomerOK() {

  LOGGER.info("======= testGetCustomerOK starting...");

  try {
    CustomerDTO targetDTO = this.serviceImpl.getCustomer("ABC99");

    // Check.
    Assert.assertNotNull("Extra not found.", targetDTO);
    Assert.assertEquals("accountIds must be the same.", "ABC99", targetDTO.getAccountId());

  } catch (CustomerBusinessException exception) {
    Assert.fail(exception.getMessage());
  } catch (UnavailableResourceException exception) {
    Assert.fail(exception.getMessage());
  } catch (UnexpectedException exception) {
    Assert.fail(exception.getMessage());
  } catch (Exception exception) {
    Assert.fail(exception.getMessage());
  }

  LOGGER.info("======= testGetCustomerOK done.");
}

Next, the catching of exceptions and calling Assert.fail() is in fact the same work that the test runner actually does. We can let the test runner handle those cases:

/**
 *
 */
@Test
public void testGetCustomerOK() throws Exception {

  LOGGER.info("======= testGetCustomerOK starting...");

  CustomerDTO targetDTO = this.serviceImpl.getCustomer("ABC99");

  // Check.
  Assert.assertNotNull("Extra not found.", targetDTO);
  Assert.assertEquals("accountIds must be the same.", "ABC99", targetDTO.getAccountId());

  LOGGER.info("======= testGetCustomerOK done.");
}

The remaining logs are actually information that can already be known from the test runner, since test results are presented by methods. My guess is that they were added to make the previous logs more readable. They can go now:

/**
 *
 */
@Test
public void testGetCustomerOK() throws Exception {
  CustomerDTO targetDTO = this.serviceImpl.getCustomer("ABC99");

  // Check.
  Assert.assertNotNull("Extra not found.", targetDTO);
  Assert.assertEquals("accountIds must be the same.", "ABC99", targetDTO.getAccountId());
}

The “check” comment serves no useful purpose. Also, there is an empty comment that obviously adds no value:

@Test
public void testGetCustomerOK() throws Exception {
  CustomerDTO targetDTO = this.serviceImpl.getCustomer("ABC99");

  Assert.assertNotNull("Extra not found.", targetDTO);
  Assert.assertEquals("accountIds must be the same.", "ABC99", targetDTO.getAccountId());
}

We are down to 6 lines of code. The first assert does a check that is also done implicitely by the following one. That seems redundant. Also, it is clearly a situation that should not happen in a normal test run. That can go:

@Test
public void testGetCustomerOK() throws Exception {
  CustomerDTO targetDTO = this.serviceImpl.getCustomer("ABC99");

  Assert.assertEquals("accountIds must be the same.", "ABC99", targetDTO.getAccountId());
}

The remaining assertion has a comment that says what the test does. In some way, there is some value in this. However, in most (all?) cases, the person seeing the error would open the test class anyway and read the code. The test comment seems redundant. At the very least, its value is far from clear. In those situations, I remove the comments:

@Test
public void testGetCustomerOK() throws Exception {
  CustomerDTO targetDTO = this.serviceImpl.getCustomer("ABC99");

  Assert.assertEquals("ABC99", targetDTO.getAccountId());
}

OK, a little inlining can now be done:

@Test
public void testGetCustomerOK() throws Exception {
  Assert.assertEquals("ABC99", this.serviceImpl.getCustomer("ABC99").getAccountId());
}

The Assert.assertEquals method can also be statically imported:

@Test
public void testGetCustomerOK() throws Exception {
  assertEquals("ABC99", this.serviceImpl.getCustomer("ABC99").getAccountId());
}

And the test could use a renaming:

@Test
public void can_obtain_a_customer_by_account_id() throws Exception {
  assertEquals("ABC99", this.serviceImpl.getCustomer("ABC99").getAccountId());
}

Hm… the “this.” clause is not really necessary:

@Test
public void can_obtain_a_customer_by_account_id() throws Exception {
  assertEquals("ABC99", serviceImpl.getCustomer("ABC99").getAccountId());
}

And… that’s it. I’m down to 4 lines of code. There might be fewer comments, but I believe the result is a lot more readable.

“Adding manpower to a late software project makes it later”. The so-called Brooks’s Law on productivity of software project is well known, since Fred Brooks’ seminal work The Mythical Man-Month, first published in 1975.

That is one of the reasons I’m wince a little when a company talks about plans to hire lots of software developers.

This recent study by QSM seems to prove me right. Its conclusion seems to be that Brooks’s Law can be extended to projects that are not late. At any rate, code is not produced at a faster pace with more manpower.

The idea was to compare the time require to build a 100,000 lines projects, depending on the size of the teal. Results are :

• 32-people teams : 8.9 months
• 4-people teams : 9.1 months

That’s a one-week difference. 2-3%!

Costs are obviously much in favor of small teams (roughly 8 times cheaper for the same result!). One of the reasons suggested could be that large teams produce many more bugs.

Unfortunately, there are no discussions on other possible explanations, such as complexity in coordinating, communication, the number of managers, location over multiple rooms or sites, skills, turnover, size of the company… I would have also liked to see other sizes compared. Is there a productivity peak at 2 people ? 6 ? 4 ?

A few years ago, I got into a discussion where one of the persons remarked: “managing large teams is hard!” As far as as I’m concerned, the answer seems to be: “well, don’t do it, then.”

Do you know what’s worse that static methods or classes marked as final? I’ll tell you what’s worse: static methods that return final classes. That only provides private constructors.

Here I was, merrily testing my way through a piece of software that sends emails. According to the Java Mail documentation, you are supposed to first create an email session as follows:

Session mailSession = Session.getInstance(properties);

(it is worth noting that all what the getInstance() method does is call the private constructor for Session: new Session(props, null)).
Had Session been a more normal class, I could have mocked it like that:

Session mockSession = mock(Session.class);

and probably checked how it is being passed around like this:

verify(someService).startEmailSession(mockSession);

What follows is what I have to do instead.

The case of static methods

If getInstance() had been an instance method, I would have been able to mock it:

Session mockSessionProvider = mock(Session.class);
Session mockSessionInstance = mock(Session.class);
when(mockSessionProvider.getInstance()).thenReturn(mockSessionInstance);

Unfortunately, a static method cannot easily be mocked with Mockito (other mocking frameworks support that, but I believe they make the tests too obscure). So, first step: create a wrapper class just for the builder method.

public class SessionProvider {
	public Session getInstance(Properties properties) {
		return Session.getInstance(properties);
	}
}

The case of final classes

So far, so (relatively) good. However, since Session is a final class, the usual mocking mecanism under Mockito does not work:

SessionProvider mockSessionProvider = mock(SessionProvider.class);
Session mockSessionInstance = mock(Session.class); // fails because Session is final
when(mockSessionProvider.getInstance()).thenReturn(mockSessionInstance);

The case of private constructors

Another option would have been to instantiate the session instance, but that fails too, since the constructor is private:

SessionProvider mockSessionProvider = mock(SessionProvider.class);
Session sessionInstance = new Session(properties, null); // fails because constructor is private
when(mockSessionProvider.getInstance()).thenReturn(sessionInstance);

A third way might have been to call the original Session.getInstance() to create the test object:

SessionProvider mockSessionProvider = mock(SessionProvider.class);
Session sessionInstance = Session.getInstance(properties);
when(mockSessionProvider.getInstance()).thenReturn(sessionInstance);

That would work, but that also opens a whole new can of worms. Session does not implement the equals() method, so if I want to check the value of the Session instance passed around, I must either:

• compare with the pointer to the sessionInstance — the issue is that there is no way to check that the session has not been changed (any call to a setter method on the session instance will have no effect on the comparison of pointers)
• implement a generic deep equal matcher for Mockito — something we did at my company, but that I am reluctant to do myself
• use an argument captor to catch the instance passed and test — which produces a fair amount of awkward lines of test
• create a custom argument matcher — which hides the comparison in a separate class or method
• yet another is to wrap the instance of Session in yet another class, such as SessionInstance — which would require us to also wrap any service that takes an instance of Session

So we also need a wrapper for the Session instance:

public class SessionInstance {
	private final Session session;

	public SessionInstance(Session session) {
		this.session = session;
	}

	public Session getSession() {
		return session;
	}
}

Which forces us to change the SessionProvider:

public class SessionProvider {
	public SessionInstance getInstance(Properties properties) {
		return new SessionInstance(Session.getInstance(properties));
	}
}

At last, we can mock this way:

SessionProvider mockSessionProvider = mock(SessionProvider.class);
SessionInstance mockSessionInstance = mock(SessionInstance.class);
when(mockSessionProvider.getInstance()).thenReturn(mockSessionInstance);

Done? Hardly. All other, possibly mockable, classes from third-parties that rely on instances of Session must now also by wrapper. For example, a third-party JavaMailTransport that requires an instance of Session now needs its own wrapper if I want to test it. Sigh…

Here is a typical example of a test method

@Test
public void should_search_by_path() {
	Searcher searcher = new Searcher();
	Path location = new Path("somewhere");
	String data = "data";

	searcher.putAt(data, location);

	assertThat(searcher.findAt(location), is(data));
}

It seems that many developers consider this good code. I don’t. My main gripe here is that the presence of variables does not add much useful information. Instead, they make the code too verbose.

A first rewrite would produce something like that:

@Test
public void should_search_by_path() {
	Searcher searcher = new Searcher();
	searcher.putAt("data", new Path("somewhere"));

	assertThat(searcher.findAt(new Path("somewhere")), is("data"));
}

Shorter, which is good. I also like the fact it makes clear that it is the value of the parameters that matter, and not their pointers.
We can go further by leveraging local static methods.

@Test
public void should_search_by_path() {
	Searcher searcher = new Searcher();
	searcher.putAt("data", path("somewhere"));

	assertThat(searcher.findAt(path("somewhere")), is("data"));
}

private static Path path(String path) {
	return new Path(path);
}

(a static method is not mandatory, but I think it makes its intention of being a utility method clearer)
And, finally, if this test class is dedicated to the Searcher class, I would probably make the searcher variable a field, as it would be used in most test methods.

private Searcher searcher = new Searcher();

@Test
public void should_search_by_path() {
	searcher.putAt("data", path("somewhere"));

	assertThat(searcher.findAt(path("somewhere")), is("data"));
}

private static Path path(String path) {
	return new Path(path);
}

This is more lines than the original example. However, the searcher variable is on top of the class and the path() method is at the bottom, far out of the way of the test methods. Indeed, I would often have several such static methods at the bottom of my classes (and, rarely, one or two more instance variables at the top). I even tend to do so when I have such a single method using them.

Beside the fact that the method has fewer lines, I also like that it makes for more fluent code. It almost reads like a story “when the searcher is being put a String of value ‘data’ at the path of name ‘somewhere’, then it should be able to assert that the String found at the path of name ‘somewhere’ has value ‘data’”.

Another example, so that my position is clear:

@Test
public void should_find_by_address() {
	String aValidAddress = "10 Downing Street";
	Person aPerson = new Person("David Cameron");
	contacts.put(aValidAddress, aPerson);

	assertThat(contacts.get(aValidAddress), is(aPerson));
}

I’d very much rewrite this as follows:

@Test
public void should_be_able_to_find__a_person_when_storing_with_a_valid_address() {
	contacts.put("10 Downing Street", person("David Cameron"));

	assertThat(contacts.get("10 Downing Street"), is(person("David Cameron")));
}

Any thoughts?

At Devoxx last week, Joshua Bloch argued during his talk “The Evolution of Java: Past, Present, and Future” that varargs are only “somewhat useful”. I think he is overlooking some usages, particularly in tests. Here is my case.

A reminder on how varargs work: essentially, they allow the last parameter of a method to be made of zero to many values of the same type.
For example, a method like this:

int max(int... values) {...};

is used by code like that:

int maximum = max(1, 2, 7, 0);
int maximum = max(1);

In truth, that form is only moderately useful in production code. It turns out that, on my projects at least, it is not so common for methods to be called with varying number of parameters. And in those cases, it is often acceptable to simply overload a method with more parameters.

There are however at least two cases where varargs shine.

One is in utilities classes. For example, Math.max() only takes exactly two parameters. Which comparisons between 3 or more elements very awkward. If a varargs had been used, Math.max(3, Math.max(1, 4)) would be written Math.max(3, 1, 4). Agreed, that does not happen that often. But it does occasionally.
(I must admit that I do not understand exactly why Math.max() has not been modified to take a varargs nowadays)

However, the biggest benefit, I believe, is in the writing of tests.
Tests tend to contain lots of variations in the values passed to the code under test. And it is critical to keep the clutter to a minimum.
For example, here is how a typical series of tests might look like

@Test
public void should_find_the_longest_name_for_a_single_user() {
	List users = new ArrayList();
	users.add(new User("eric"));
	assertThat(findLongestName(users), is("eric"));
}

@Test
public void should_find_the_longest_name_when_shorted_is_first() {
	List users = new ArrayList();
	users.add(new User("eric"));
	users.add(new User("cecile"));
	assertThat(findLongestName(users), is("cecile"));
}

@Test
public void should_find_the_longest_name_when_longest_is_first() {
	List users = new ArrayList();
	users.add(new User("cecile"));
	users.add(new User("eric"));
	assertThat(findLongestName(users), is("cecile"));
}

@Test(expected = RuntimeException.class)
public void should_fail_when_search_for_the_longest_name_with_no_users() {
	assertThat(findLongestName(new ArrayList()), is("eric"));
}

Of course, they can be made a bit nicer using the varargs in Arrays.asList():

@Test
public void should_find_the_longest_name_for_a_single_user() {
	assertThat(findLongestName(asList(new User("eric"))), is("eric"));
}

@Test
public void should_find_the_longest_name_when_shorted_is_first() {
	assertThat(findLongestName(asList(new User("eric"),
		new User("cecile"))), is("cecile"));
}

@Test
public void should_find_the_longest_name_when_longest_is_first() {
	assertThat(findLongestName(asList(new User("cecile"),
		new User("eric"))), is("cecile"));
}

@Test(expected = RuntimeException.class)
public void should_fail_when_search_for_the_longest_name_with_no_users() {
	findLongestName(Arrays.asList());
}

However, the real goodness comes when writing our own builder method:

private static List users(String... names) {
	List users = new ArrayList();
	for (String name : names) {
		users.add(new User(name));
	}
	return users;
}

(side note: I like this type of methods to be private -so that I get notified when they are not used anymore- and static -mostly because they look nicer in italic, which makes it clearer that they are not part of the code being tested-)

which allow our tests to become:

@Test
public void should_find_the_longest_name_for_a_single_user() {
	assertThat(findLongestName(users("eric")), is("eric"));
}

@Test
public void should_find_the_longest_name_when_shorted_is_first() {
	assertThat(findLongestName(users("eric", "cecile")), is("cecile"));
}

@Test
public void should_find_the_longest_name_when_longest_is_first() {
	assertThat(findLongestName(users("cecile", "eric")), is("cecile"));
}

@Test(expected = RuntimeException.class)
public void should_fail_when_search_for_the_longest_name_with_no_users() {
	findLongestName(users());
}

Tests become a lot shorter, consistent, and easier to read. In fact, at this point, it is worth grouping the tests (at least those that do not throw an exception) into a single one:

@Test
public void should_find_the_longest_name_for_a_single_user() {
	assertThat(findLongestName(users("eric")), is("eric"));
	assertThat(findLongestName(users("eric", "cecile")), is("cecile"));
	assertThat(findLongestName(users("cecile", "eric")), is("cecile"));
}

@Test(expected = RuntimeException.class)
public void should_fail_when_search_for_the_longest_name_with_no_users() {
	findLongestName(users());
}

Final note: I prefer not to group those builder methods into a general utilities class — I tend to duplicate them in each test class. One reason is that I prefer keeping much of the test code close together. Another is that I tend to change the name of the method from test class to test class to make the code more fluent. Also, I think that DRY principles are not as important in the tests as they are in the production code.