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.

Advertisements

Author: tamasgyorfi

Senior software engineer, certified enterprise architect and certified Scrum master. Feel free to connect on Twitter: @tamasgyorfi

1 thought on “XML operations – a refactoring dojo”

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s