Mocking tutorial. Part 2: Mockito

You might remember from the previous post that we are discovering the possibilities of different mocking frameworks. In the first part of this post series, I was presenting several features of JMock. In the second part we are going to see how those features are similar/different in another popular mocking framework, namely Mockito.

The example through which we exemplify the soul Mockito, is the very same we used for JMock. If you -somehow- don’t remember our example code, then you can find it below:

public class DocumentReader {

	private final Logger logger;
	private final Document document;

	public DocumentReader(Logger logger, Document document) {
		this.logger = logger;
		this.document = document;

	}

	public DocumentContent getWholeDocument() {
		DocumentContent documentContent = null;
		logger.log(Level.SEVERE, "getWholeDocument got called");
		document.openDocument();
		try {
			documentContent = readDocumentSections();
		} catch (DocumentException documentException) {
			logger.log(Level.SEVERE, "DocumentException caught in documentReader.getWholeDocument(Document)");
		} finally {
			document.closeDocument();
			logger.log(Level.SEVERE, "getWholeDocument left");
		}
		return documentContent;
	}

	private DocumentContent readDocumentSections() throws DocumentException {
		DocumentContent documentContent  = new DocumentContent(document.getDocumentTitle());
		int sectionCounter = 0;
		while (!document.documentEndReached()) {
			Section currentSection = document.getSectionBySectionNumber(sectionCounter++);
			documentContent.addSection(currentSection);
		}
		return documentContent;
	}

	public String getDocumentTitle() {
		return document.getDocumentTitle();
	}
}

Document is an empty class, it does not do anything, just defines some methods:

package impl;

public class Document {

	private final String path;

	public Document(final String path){
		this.path = path;

	}

	public void openDocument() {
		//...
	}

	public boolean documentExists() {
		//...
		return true;
	}

	public void closeDocument() {
		//...
	}

	public Section getSectionBySectionNumber(final int sectionNumber) throws DocumentException{
		//...
		return new Section();
	}

	public boolean documentEndReached() {
		return false;
	}

	public String getDocumentTitle() {
		return "Title";
	}
}

DocumentContent is just a simple data holder class:

package impl;

import java.util.ArrayList;
import java.util.List;

public class DocumentContent {

	private final String documentTitle;
	private List<Section>sections = new ArrayList<Section>();

 public DocumentContent(String documentTitle) {
 this.documentTitle = documentTitle;
 }

 public void addSection(Section newSection) {
 sections.add(newSection);
 }
}

All the other referenced classes are empty.

Now, as TDD believers, we can create a new JUnit test case. Let’s call it DocumentReaderTestMockito, in order to be consistent with the naming convention used in the previous post. After the creation of the new file, -as a first step- we have to let the test runner know that we are using Mockito. It is quite similar to what we’ve done in the last post; namely annotate the class with the @RunWith(MockitoJUnitRunner.class) annotation. After this step, you can have something like this:

@RunWith(MockitoJUnitRunner.class)
public class DocumentReaderTestMockito {

}

When using Mockito, you don’t need to create a special object, through which you can create mock object. These mocks can simply be injected automatically, using the @Mock annotation. Remember that you have to annotate your test class with the @RunWith annotation, or the automatical mock injection will not be working.
So, in order to create mock objects with Mockito, you simply need to have something like this:

@RunWith(MockitoJUnitRunner.class)
public class DocumentReaderTestMockito {

	@Mock
	Document document;
	@Mock
	Logger logger;
}

If this was a contest of user-friendliness, Mockito would have the lead over JUnit (that’s it; Mockito is simpler from this point of view). Now, we can write a test case, which (like at the last time) throws an exception when the second Section object is being read. In order to do something like that, you might have something like this:

@RunWith(MockitoJUnitRunner.class)
public class DocumentReaderTestMockito {

	@Mock
	Document document;
	@Mock
	Logger logger;

	@Test
	public void shouldThrowExceptionWhileReadingDocument() throws DocumentException {
		DocumentReader documentReader = new DocumentReader(logger, document);
		when(document.getSectionBySectionNumber(2)).thenThrow(new DocumentException());
		documentReader.getWholeDocument();
	}
}

Quite simple, huh? We just create our mock objects using dependency injection, pass them to the class under test’s instance, and tell Mockito when we want the exception to be thrown. If you look at the code, you may spot that the only Mockito-realted line inside the test case is the following: when(document.getSectionBySectionNumber(2)).thenThrow(new DocumentException());. I don’t think this line needs further comments. It is kind of straight forward. One thing to remember -exactly as in the case of JMock – Mockito returns default objects for method calls. This means zeroes, empty collections and nulls – depending on the return type of the method.

As you may have already noticed, Mockito doesn’t require you to take care of all the method calls on your mock objects. You just tell the desired interactions and you are done. Of course, if you want Mockito to return specific values, you can do that. Let’s take an example. The following test case will test the normal execution flow; the document’s end will be reached at the first call to documentEndReached().

	@Test
	public void shouldThrowExceptionWhileReadingDocument()
			throws DocumentException {
		DocumentReader documentReader = new DocumentReader(logger, document);
		when(document.documentEndReached()).thenReturn(true);

		documentReader.getWholeDocument();
	}

Or, if you want to be sure that the code is working correctly (e.g. we only leave the loop when the document’s end is reached), you just replace the “when” above, with something like this:

when(document.documentEndReached()).thenReturn(false, false, true);

Which is: for the first two invocations of documentEndReached(), we return false, while for the third one we return a true (so we leave the loop).

You might remember the concept of cardinality from JMock. There is a way you can make Mockito care about the number of invocations on your mock objects. This is what the method verify() is used for. Let’s take an example:

	@Test
	public void shouldThrowExceptionWhileReadingDocument()
			throws DocumentException {
		DocumentReader documentReader = new DocumentReader(logger, document);
		when(document.documentEndReached()).thenReturn(false, false, true);
		documentReader.getWholeDocument();

		verify(logger).log(Level.SEVERE, "getWholeDocument got called");
		verify(logger).log(Level.SEVERE, "getWholeDocument left");
	}

Now, if any of the invocations are not happening, our test case will fail. Of course, you may compact the two verify statements into one, if you don’t really care about the log messages. Mockito (just like JMock) defines the concept of argument matcher. Let’s just rewrite the test case to something like this:

	BaseMatcher<Level> isLevel = new BaseMatcher<Level>() {
		public boolean matches(Object item) {
			return item instanceof Level;
		}
		
		@Override
		public void describeTo(org.hamcrest.Description description) {
		}
	};

	@Test
	public void shouldThrowExceptionWhileReadingDocument()
			throws DocumentException {
		DocumentReader documentReader = new DocumentReader(logger, document);
		when(document.documentEndReached()).thenReturn(false, false, true);
		documentReader.getWholeDocument();

		verify(logger, times(2)).log(argThat(isLevel), anyString());
	}

When using Mockito, you can match any kind of object through the method argThat(). In order to be able to match a certain type of object (Level, in our case), you have to define your own matcher. You just simply overwrite the matches() method of the BaseMatcher abstract class, and pass the newly created object to argThat() method as a parameter. There are several predefined matchers too, like anyString(), anyInt(), anySet(), and many many others.

As you can see we’ve also passed a second parameter to the verify() method, that will tell Mockito that we want that certain invocation to happen twice. times(1) is the default value, and it can be omitted like we did before.

As a final example, let’s write a test case that will define all the invocations we need (and for that let’s get back to our first example, the one which cares for exception throwing. Such a way we can compare the final solutions implemented with Mockito and JMock):

	BaseMatcher<Level> isLevel = new BaseMatcher<Level>() {
		public boolean matches(Object item) {
			return item instanceof Level;
		}
		
		@Override
		public void describeTo(org.hamcrest.Description description) {
		}
	};

	@Test
	public void shouldThrowExceptionWhileReadingDocument()
			throws DocumentException {
		DocumentReader documentReader = new DocumentReader(logger, document);
		when(document.getSectionBySectionNumber(2)).thenThrow(new DocumentException());
		
		documentReader.getWholeDocument();

		verify(logger, times(3)).log(argThat(isLevel), anyString());
		verify(document).openDocument();
		verify(document).getDocumentTitle();
		verify(document, times(2)).documentEndReached();
		verify(document, times(2)).getSectionBySectionNumber(anyInt());
		verify(document).closeDocument();
	}

Taking into account the matcher we had to write, we end up with nearly the same complexity as in the previous post. However, Mockito would not notify you if you left out any of the interaction (it would, however, fail the test case if you defined the wrong number of invocations).

Now you have an example with both Mockito and JMock, so pick your favorite and have fun.

Mocking tutorial. Part 1: JMock

Today I held a mock dojo (one and a half hour) in order to introduce mocking and JMock to my audience. This post is intended to be the first part, which focuses exclusively on JMock. In the second part (I don’t know when I’ll have the time for that) I will go through the very same exercise but with Mockito.

First of all, what is mocking? Well, in the most cases a class has dependencies. Those dependencies have to be somehow broken, so we can test one and only one class at a time. In order to break the dependencies, we have several possibilities; dummies, fakes, stubs and finally mocks. Mocks are special cases of test doubles, because one has to predefine expectations on them; these expectations form a list of calls expected to be received by the mock object. Trough mocking we can isolate all the dependencies of a class, so it can be tested in isolation, without testing the functionality of the dependent classes (remember these are unit tests). For a more accurate introduction of mock objects (which is outside the scope of this post), please be referred to this article: http://en.wikipedia.org/wiki/Mock_object

In order to exemplify the whole process of mocking, we will use the following code (I know its has its issues, but anyway it’s gonna be useful to guide you through the process of mocking):

public class DocumentReader {

	private final Logger logger;
	private final Document document;

	public DocumentReader(Logger logger, Document document) {
		this.logger = logger;
		this.document = document;

	}

	public DocumentContent getWholeDocument() {
		DocumentContent documentContent = null;
		logger.log(Level.SEVERE, "getWholeDocument got called");
		document.openDocument();
		try {
			documentContent = readDocumentSections();
		} catch (DocumentException documentException) {
			logger.log(Level.SEVERE, "DocumentException caught in DocumentReader.getWholeDocument(Document)");
		} finally {
			document.closeDocument();
			logger.log(Level.SEVERE, "getWholeDocument left");
		}
		return documentContent;
	}

	private DocumentContent readDocumentSections() throws DocumentException {
		DocumentContent documentContent  = new DocumentContent(document.getDocumentTitle());
		int sectionCounter = 0;
		while (!document.documentEndReached()) {
			Section currentSection = document.getSectionBySectionNumber(sectionCounter++);
			documentContent.addSection(currentSection);
		}
		return documentContent;
	}

	public String getDocumentTitle() {
		return document.getDocumentTitle();
	}
}

Document is an empty class, it does not do anything, just defines some methods:

package impl;

public class Document {

	private final String path;

	public Document(final String path){
		this.path = path;

	}

	public void openDocument() {
		//...
	}

	public boolean documentExists() {
		//...
		return true;
	}

	public void closeDocument() {
		//...
	}

	public Section getSectionBySectionNumber(final int sectionNumber) throws DocumentException{
		//...
		return new Section();
	}

	public boolean documentEndReached() {
		return false;
	}

	public String getDocumentTitle() {
		return "Title";
	}
}

DocumentContent is just a simple data holder class:

package impl;

import java.util.ArrayList;
import java.util.List;

public class DocumentContent {

	private final String documentTitle;
	private List
<section>sections = new ArrayList
<section>();

 public DocumentContent(String documentTitle) {
 this.documentTitle = documentTitle;
 }

 public void addSection(Section newSection) {
 sections.add(newSection);
 }
}

All the other referenced classes are empty. You just create them with an empty body, and it should be OK.
Now, we are ready to create a test case. JUnit4 should be put onto your build path, and JMock jars too. Your test case may be called DocumentReaderTestJMock.

OK, the first thing when you want to use JMock, is to create a JUnit4Mockery and tell the test runner that you are actually using JMock. I usually call these mockeries either mockery, or context. In order to create such a mockery you may want to have something like this:

@RunWith(JMock.class)
public class DocumentReaderTestJMock {

	JUnit4Mockery context = new JUnit4Mockery();
}

Now we have JMock as testrunner (see the annotation of the class) and we’ve also got a mockery. So, we could just mock any type of class, right? Wrong! JMock has originally been designed to be able to mock interfaces only. Should you try mocking a class right now, you’d get an error message telling you that only interfaces can be mocked. Fortunately, JMock defines an extension so it can mock classes too. The resulting declaration is not a beautiful one, but anyway, let’s just see how it works:

@RunWith(JMock.class)
public class DocumentReaderTestJMock {

	JUnit4Mockery context = new JUnit4Mockery() {
		{
			setImposteriser(ClassImposteriser.INSTANCE);
		}
	};
}

OK, now we are really ready to mock classes. Let’s just try it, and create a test case, which will test the exceptional path. In order to do that, we need to have an exception to be thrown while reading document content. In order to have an extra constraint, let’s suppose that our exception is thrown while reading the second section. So, we end up with something like this:

@RunWith(JMock.class)
public class DocumentReaderTestJMock {

	JUnit4Mockery context = new JUnit4Mockery() {
		{
			setImposteriser(ClassImposteriser.INSTANCE);
		}
	};

	final Document document = context.mock(Document.class);
	final Logger logger = context.mock(Logger.class);

	@Test
	public void shouldThrowExceptionWhileReadingDocument()
			throws DocumentException {
		DocumentReader documentReader = new DocumentReader(logger, document);
		final Section section = new Section();

		context.checking(new Expectations() {

			{
				allowing(logger).log(with(any(Level.class)), with(any(String.class)));
				oneOf(document).openDocument();

				oneOf(document).getDocumentTitle();
				will(returnValue("Title"));

				exactly(2).of(document).documentEndReached();
				will(returnValue(false));

				exactly(2).of(document).getSectionBySectionNumber(with(any(int.class)));
				will(onConsecutiveCalls(returnValue(section),
						        throwException(new DocumentException())));

				oneOf(document).closeDocument();
			}
		});

		documentReader.getWholeDocument();
	}
}

Now let’s go through the test case. As you can see, JMock can mock objects through the already-well-known context object. We have two mock objects, a Logger and a Document. Whenever you have such a mock object in JMock, you have to take care of the invocations on that particular mock. This means that you have to enumerate all the method calls on the mock object, along with their parameters and the number of times the methods are called (if you forget any of them, your test case will fail). This part is represented by the anonymous inner class (context.checking); Those instructions inside it have the following structure:

1.) A cardinality number and a mock object as its parameter: oneOf(mockObject), exactly(2).of(mockObject), allowing(mockObject) : By this you can tell how many times a particular method is called on a particular mock object. oneOf means that the invocation is called once and only once, exactly(2) is straightforward, while allowing() means any number of invocations. Other cardinality numbers and values: atLeast(x).of(mockObject), atMost(x).of(mockObject), never(mockObject), ignoring(mockObject). Take care when using the last one: this will completely ignore your mock object!

2.) The method called on the mock object with its parameters. These parameters can be defined two ways:

  • exact values (But watch out when passing in plain old java objects. Those objects won’t be matched unless you have defined an accurate equals() method).
  • Parameter matchers: with(any(Something.class)), with(equal(“Hello World”)). Take care! You cannot mix exact values and matchers inside an expectation. E.g. oneOf(calculatorMock).add(1, with(any(int.class))) will be invalid. It will throw an exception telling you that not all parameters were supplied in the form of matchers. Either use exact values or matchers only. If you are sure about one (or several) parameters only, you have to use the following construct: oneOf(calculatorMock).add(with(equal(1)), with(any(int.class))).

Note that this expectation:

allowing(logger).log(with(any(Level.class)), with(any(String.class)));

could be turned into a more strict one:

exactly(3).of(logger).log(with(equal(Level.SEVERE)), with(any(String.class)));

or if you don’t care about the logger at all, then you can have :

ignoring(logger);

3.) Return statement. JMock can return default values for method calls, but if you want to customize these returned values, of course yo cant. That’s what will(returnValue(aValue)) is used for. If the value you want the function to return  is not compatible with the return type of the method, you -again- get a runtime exception (It says something like tried to return AAA from a method that can only return BBB). Note that you can make JMock simulate that an exception was thrown, by using the will(throwException(new MyException())).

You can observe a very nice functionality of JMock, by looking at the lines:

exactly(2).of(document).getSectionBySectionNumber(with(any(int.class)));
will(onConsecutiveCalls(returnValue(section), throwException(new DocumentException())));

It says that getSectionBySectionNumber() is called exactly two times. When it is called for the first time a section object will be returned, while for the second time an exception will be thrown. This way you don’t have to duplicate code lines, just customize the desired return statements.

Note that the order in which invocations are arranged inside the expectation block doesn’t matter (there is a way -by using the inSequance construct- through which you can force the test runner care about the order of expectations). However, it is necessary to define the expectations before the method under test is called.

I get it all the time that “it is too tiring to write all those expectations with all the correct parameters and all the correct cardinality “. However, I think, it is actually a very good feature. There are cases when a test case can not test anything else but collaborations and, optionally the order in which methods were called. Besides, it happened several times, that I figured out performance issues thanks to JMock cardinalities. When I had to introduce an expectation like exactly(9).of(mockObject) it made clear that resource handling is somehow not efficient.

The rest of the class can be tested similarly, requiring no additional knowledge than presented above. So, be my guest, try to achieve 100% coverage.

Code Kata Nr. 5

This week we returned to the world of strings and realistic programming (more or less). The kata itself is a slightly modified variant of the well known Soundex algorithm. (see a detailed description of Soundex algorithm here: http://en.wikipedia.org/wiki/Soundex)

And now the actual exercise we had:

  • The algorithm receives a name as input, which is at least two characters long
  • Retain the first letter of the name, and drop all occurrences of a, e, h, i, o, u, w, y in other positions.
  • If two or more letters are the same on adjacent positions in the name, omit all but the first.
  • Here we get a little tricky and diverge from the original algorithm: convert all the resulting string to all-uppercase
  • Convert all letters (but the first) to ASCII characters
  • Back to the original algorithm: convert the resulting string to the form LETTER NUMBER NUMBER NUMBER (note that NUMBER is an ASCII character of two digits)
  • If there are not enough characters in the string, use the number “00” instead

Why was it needed to modify the algorithm to something that does not really do anything useful? Well it was required because of the nature of the katas; we play them the following way:

  1. The problem is presented
  2. We have a first round of x minutes, when the players can implement their solution employing TDD
  3. After that we have a discussion round when everyone tells the others what and how did they do
  4. All the solutions are erased
  5. A second round comes of x minutes again, where everyone can use ideas or parts of others’ solution

This kata was played with 20 minutes long rounds. If the character grouping had been a constraint, we would have spent all the time testing that all the characters are grouped properly. The checking of the banned characters took too much time anyway. If you play katas differently (longer rounds, or no rounds at all, no time constraint), then just add this requirement too, and implement the whole algorithm (have fun and see its limitations :-)).

There was no new constraint introduced, but all the previous ones remained valid.

JUnit exception handling

I was browsing through some test classes lately and saw some ways of handling exceptions in test code. It seems to me that different versions are used randomly; there is no common style, a pattern that everyone follows. So I’d like to share some good and not-so-good solutions.

In order to exemplify exception handling, I’ll use a very simple example: Let’s consider a primitive message sender, that is able to send a text message to a given IP address. The exception we are hunting for is called ExpectedException, and there’s nothing interesting about it. It’s just a simple exception:

package expectedexception;

public class ExpectedException extends Exception{

	private static final long serialVersionUID = 1L;

	public ExpectedException(String message) {
		super(message);
	}
}

Ok, so a system that sends messages to different IP addresses, huh? Then we might need something to check the validity of those addresses, it seems. (Usually I don’t write static methods, but in this case I used one for brevity):

package expectedexception;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

public class IPAddressValidator {

	private static final String IP_ADDRESS_STRUCTURE = "\\d{1,3}+\\.\\d{1,3}+\\.\\d{1,3}+\\.\\d{1,3}+";

	public static boolean isValidIpAddress(String ipAddress) {
		Pattern ipAddressPattern = Pattern.compile(IP_ADDRESS_STRUCTURE);
		Matcher matcher = ipAddressPattern.matcher(ipAddress);
		if (matcher.matches()) {
			return matcher.hitEnd();
		}
		return false;
	}
}

Now let’s see the message sender class. It’s code is not very complicated (note the null pointer exception when adding an element to an uninitialized list!):

package expectedexception;
import java.util.List;

public class MessageSender {

private List sentMessages;

    public void sendMessage(String ipAddress, String message) throws ExpectedException {
        if (IPAddressValidator.isValidIpAddress(ipAddress)) {
            sentMessages.add(message);
            //...
        } else {
            throw new ExpectedException("Hello world!');
        }
    }
}

The first approach we’ll be analyzing looks like:

	@Test(expected = Exception.class)
	public void shouldThrowExceptionWhenIpAddressIsNotValid() throws Exception {
		MessageSender messageSender = new MessageSender();

		messageSender.sendMessage("128.192.1.45", "Hello World");
	}

So we run the test, and get a green bar! If one is not sharp-eyed enough, will just go on and miss the point that the code is not working. The IP address is perfectly valid (it happens at copy-paste programming that someone just forgets to supply the correct parameters), so our message (Hello World) should be sent to that IP address. However, as we pointed out, there is a NullPointerException thrown inside the if statement; NullPointerException is a subclass of Exception, and as such, it perfectly satisfies the test condition. In short, should any kind of exception be thrown inside the method under test, the test case remains green. Never, never expect raw exception as result for a test case.

Ok, let’s suppose we corrected the null pointer from the example above, and so we have (note that “Ip Addres” is misspelled on purpose 🙂 ):

package expectedexception;
import java.util.List;

public class MessageSender {

	public void sendMessage(String ipAddress, String message) throws ExpectedException {
		if (IPAddressValidator.isValidIpAddress(ipAddress)) {
			//...
		} else {
			throw new ExpectedException("Ip Addres not valid");
		}
	}
}

Another approach I saw, is to write a test case that looks like:

	@Test
	public void shouldThrowExceptionWhenIpAddressIsNotValid() throws Exception {
		MessageSender messageSender = new MessageSender();

		try {
			messageSender.sendMessage("128.192.1.45.7.1", "Hello World");
		} catch (ExpectedException e) {
			assertEquals("Ip Addres not valid", e.getMessage());
		}
	}

(We fond out that the test was provided a correct parameter – e.g. valid IP address. As we are handling special cases, we just simply break that value)
Ok, this test case is a little bit better than the one before. However, it is fragile as hell. What if someone comes along and spots the misspelling? In such cases one would correct that message to “IP Address not valid”. There, the test case has just been broken! Test cases should never test exception messages. In TDD, names, messages and other stuff are refactored quite many times. I do mind correcting test cases all the time when such a refactoring is carried out. A test case should test state and/or behavior and not plain text.

Let’s move on to the next approach.

	@Test
	public void shouldThrowExceptionWhenIpAddressIsNotValid() throws Exception {
		MessageSender messageSender = new MessageSender();

		try {
			messageSender.sendMessage("128.192.1.45.7", "Hello World");
			Assert.fail("IP Address validation is broken!");
		} catch (ExpectedException e) {
		}
	}

Well this one is a little bit tricky. It calls the method under test and (as we are testing that an exception is thrown); if everything goes ok, e.g. no exception is thrown, it fails the test case. The static method fail() is part of the Assert class; optionally you can pass in a message as a parameter, so it will be displayed if the test case fails. If an exception is thrown, we are handling it with an empty catch block. The caught exception will not fail the test case, so we get a green bar. This type of construction is much more useful and less fragile then the ones above.

And finally the last one:

	@Test(expected = ExpectedException.class)
	public void shouldThrowExceptionWhenIpAddressIsNotValid() throws Exception {
		MessageSender messageSender = new MessageSender();

		messageSender.sendMessage("128.192.1.45.7", "Hello World");
	}

Simple construction, no empty catch blocks. However, you have to take extra care to expect a special enough exception class. It works exactly as the test case above, but its a much compact solution.

So pick your favorite (the third or the fourth one 🙂 ), and start using it consistently.

No booleans as parameters, please

Ok, first of all; why are boolean parameters bad?

Because they are not input data to the method, but control values. This means that someone from the outside (e.g. the upper level method that calls yours) has explicit control over the method: It simply tells the function what to do. It’s like someone talks to you like: “Draw a smiley face USING A PENCIL” and after that “Sign this paper WITH YOUR PEN”.

You probably know how a drawing is usually created, and you don’t need anyone to tell you what tool to use. The same is true for the signature. There’s a very good example which is usually used for explaining encapsulation: “I don’t need to know how an engine works in order to be able to drive a car”. It’s similar with methods. The caller should not know how the called method achieves a certain goal; it knows that the lower method CAN achieve that goal and that’s it.

They’re also bad because they make your code unreadable. You see that you have to pass  boolean value to a method, but you have no idea what is it used for. So you go to the implementation and check it, wasting precious seconds (or even minutes, if the code is complicated enough)

So, how do we get rid of such methods? Simple. Write two functions:

  1. copy your original function, and give it a self-explanatory name
  2. eliminate the parameter from the newly created method. Replace all the occurrences of the parameter with the value ‘true’
  3. replace the old method call with the new one everywhere where the method was called with true
  4. now rename your original method
  5. eliminate the parameter and replace it with the value ‘false’
  6. solve your compilation problems
  7. refactor the new methods, simplifying boolean expressions (remember? you have trues and falses explicitly hardwired into your code. Those should be eliminated), extracting the common parts and renaming if its needed
  8. be happy, you’ve got rid of your control values

If your booleans are getting out of control, you can try to consider introducing polymorphism. That may lead to even more beautiful design.

Code Kata Nr. 4

This week we got back to realistic programming. After last week’s fibo-emirp numbers (even if there were two almost perfect solutions) I found out that real life problems are somehow closer to the teams’ heart.

This week we had IBAN number checking as a kata (yeah, we are doing such exercises on purpose: I’m planning a great attack on World Bank 🙂 ). In order to check an IBAN number, one has to follow the steps below (note, I am using the following IBAN as test data: “IT60Q0123412345000000753XYZ”, taken from http://www.morfoedro.it/doc.php?n=219&lang=en ):

  1. An IBAN number is an alphanumeric literal of at least five, at most thirty-four characters
  2. The alphanumeric string can only contain uppercase letters and digits
  3. The first two characters have to be LETTERS ( trough which we can identify the country of origin, IT60Q0123412345000000753XYZ, we have Italy)
  4. characters three and four have to be digits (IT60Q0123412345000000753XYZ, we’re doing great)
  5. the rest of the characters can be either letters or digits
  6. the first four alphanumeric characters have to be moved to the end of the string (Q0123412345000000753XYZIT60)
  7. all the letters have to be converted to numbers, following the formula:  A=10, B=11, C=12 … (260123412345000000753333435182960)
  8. as a last step the following computation has to be carried out; resulting number MODULO 97. If the value of the modulo is 1, then we have a valid IBAN. (*)

(*) A possible way of  modulo computation: divide the number into parts of (let’s say) eight digits. Take the modulo of this smaller number and put the result of the next pack of digits. Let us break this numerical string into five parts: 26012341, 23450000, 00753333, 43518296 and 0. The remainder of the division of 26012341 by 97 is 45. The remainder of the division of 4523450000 by 97 is 15. The remainder of the division of 1500753333 by 97 is 82. The remainder of the division of 8243518296 by 97 is 68. The remainder of the division of 680 by 97 is 1. [http://www.morfoedro.it/doc.php?n=219&lang=en]

If you’re curious of my solution, you can find it here:  https://github.com/tamasgyorfi/Code-kata—IBAN-numbers

The kata is of two rounds, twenty-five minutes each. The exercise involves a constraint also, namely the one testcase one assert. This was needed because there were many cases when I saw testcases with 5-10 asserts. In my opinion that is not a very good practice.  Let’s consider the following situation: there is an algorithm of some kind that checks numbers against some criteria. Let’s say the algorithm is faulty and fails for 1, 11, 111 etc. If there is a single test case that contains a lot of asserts (positives and negatives alternatively) it’s gonna fail for one and the test runner will not go on. Which means that the method will not be checked for 11, 111 etc. So one tries to fix the algorithm for 1, and will not notice the pattern… I personally like to see how many test cases there were, how many of them passed, how many of them failed. I don’t like asserts that are not run at all. Comments on this are welcome.

XML operations – a refactoring dojo

The katas we’ve had lately made it clear that our teams need to be introduced to several refactoring concepts. I decided that I would take some not-that-good-of-quality code, and organize a dojo so we can refactor it. It’s not that easy to find bad code on the internet, and it’s not that easy to come up with a badly implemented project either. I wasted some days searching, but then BANG! I came across this site: http://refactormycode.com/refactorings/recent/java. Actually this is a very strange page. People post bad code there, someone comes along, refactors the whole thing, and posts it back. For free! Some people are either really committed to the community or just have a lot of free time. Anyway…

The code I’ve taken from that site (and made it even worse a bit) looks like:

/*
 * Constraint: parses input strings in the form:
 * <xml><SvcRsData>a<![CDATA[<sender>John & Smith</sender>]]></SvcRsData></xml>
 */
public class StringFunctions {

	private static String string = "";

	public StringFunctions(String string) {
		this.string = string;
		doProcess(string);
	}

	public static String doProcess(String string) {
		int index = string.indexOf("<SvcRsData>");
		if (index > 0) {
			int index1 = index + 11;
			int index2 = string.indexOf("</SvcRsData>");
			String s1 = string.substring(0, index1);
			String s2 = string.substring(index2);

			return doIt(new StringBuffer(s1).append(
					escapeCDataCharacters(string.substring(index1, index2)))
					.append(s2).toString(), false);
		}

		return doIt(string, false);
	}

	private static String escapeCDataCharacters(String string) {
		StringBuffer sb = new StringBuffer();
		int index = string.indexOf("<![CDATA[");
		String myString = string;
		while (index > 0) {
			int cIndex1 = index + 9;
			int cIndex2 = myString
					.indexOf("]]>");

			sb.append(
					myString.substring(0,
							index)).append(
					doIt(myString.substring(
							cIndex1, cIndex2), true));

			myString = myString
					.substring(cIndex2 + 3);
			index = myString
					.indexOf("<![CDATA[");
		}
		return sb.append(
				myString).toString();
	}

	private static String doIt(String s, boolean extendedCharacterEscape) {
		StringBuffer retval = new StringBuffer();
		short character;
		for (int i = 0; i < s.length(); i++) {
			character = (short) s.charAt(i);
			if (character > 128 ? true
					: extendedCharacterEscape ? (character == 34
							|| character == 38 || character == 60 || character == 62)
							: false) {
				retval.append("&#");
				retval.append(character);
				retval.append(';');
			} else {
				retval.append((char) character);
			}
		}
		return retval.toString();
	}
}

So basically that’s it. We have a great dojo material here, it seems. There are no tests, the names are not that self-explanatory, methods are doing more things than they should be, and simply it’s messy. Refactoring, here we go. Tehre is a lot of work to do here, so let’s not waste time, start working! Adding test cases that cover the code is at least half an hour, another one, one and a half for refactoring.

I’ve ended up with something like this: https://github.com/tamasgyorfi/Refactoring-Dojo; It might seem to be an overkill having four different classes plus one more for the tests, but anyway, this way I could encourage my audience writing small methods and classes (so they won’t be afraid of decomposing logics to different abstraction levels). Unfortunately, I did not take into account the time needed for adding test cases so we had a dojo of one hour only. Never mind, will finish it in a second part.