Platinum Partner
java,agile,theory,defensive programming,offensive programming,paranoid programming,offensive,defense,paranoid

Offensive Programming

How to make your code more concise and well-behaved at the same time

Have you ever had an application that just behaved plain weird? You know, you click a button and nothing happens. Or the screen all of the sudden turns blank. Or the application get into a “strange state” and you have to restart it for things to start working again.

If you’ve experienced this, you have probably been the victim of a particular form of defensive programming which I would like to call “paranoid programming.” A defensive person is guarded and reasoned. A paranoid person is afraid and acts in strange ways. In this article, I will offer an alternative approach: “Offensive” programming.

The Cautious Reader

What might such paranoid programming look like? Here’s a typical example in Java:

public String badlyImplementedGetData(String urlAsString) {
	// Convert the string URL into a real URL
	URL url = null;
	try {
		url = new URL(urlAsString);
	} catch (MalformedURLException e) {
		logger.error("Malformed URL", e);
	}
 
	// Open the connection to the server
	HttpURLConnection connection = null;
	try {
		connection = (HttpURLConnection) url.openConnection();
	} catch (IOException e) {
		logger.error("Could not connect to " + url, e);
	}
 
	// Read the data from the connection
	StringBuilder builder = new StringBuilder();
	try (BufferedReader reader = new BufferedReader(new InputStreamReader(connection.getInputStream()))) {
		String line;
		while ((line = reader.readLine()) != null) {
			builder.append(line);
		}
	} catch (Exception e) {
		logger.error("Failed to read data from " + url, e);
	}
	return builder.toString();
}

This code simply reads the contents of a URL as a string. A surprising amount of code to do a very simple task, but such is Java.

What’s wrong with this code? The code seems to handle all the possible errors that may occur, but it does so in a horrible way: It simply ignores them and continues. This practice is implicitly encouraged by Java’s checked examples (a profoundly bad invention), but other languages see similar behavior.

What happens if something goes wrong:

  • If the URL that’s passed in is an invalid URL (e.g. “http//..” instead of “http://…”), the following line runs into a NullPointerException: connection = (HttpURLConnection) url.openConnection();. At this point in time, the poor developer who gets the error report has lost all the context of the original error and we don’t even know which URL caused the problem.
  • If the web site in question doesn’t exist, the situation is much, much worse: The method will return an empty string. Why? The result of StringBuilder builder = new StringBuilder(); will still be returned from the method.

Some developers argue that code like this is good, because our application won’t crash. I would argue that there are worse things that could happen than our application crashing. In this case, the error will simply cause wrong behavior without any explanation. The screen may be blank, for example, but the application reports no error.

Let’s look at the code rewritten in an offensive way:

public String getData(String url) throws IOException {
	HttpURLConnection connection = (HttpURLConnection) new URL(url).openConnection();
 
	// Read the data from the connection
	StringBuilder builder = new StringBuilder();
	try (BufferedReader reader = new BufferedReader(new InputStreamReader(connection.getInputStream()))) {
		String line;
		while ((line = reader.readLine()) != null) {
			builder.append(line);
		}
	}
	return builder.toString();
}

The throws IOException statement (necessary in Java, but no other language I know of) indicates that this method can fail and that the calling method must be prepared to handle this.

This code is more concise and if there is an error, the user and log will (presumably) get a proper error message.

Lesson #1: Don’t handle exceptions locally.

The Protective Thread

So how should this sort of error be handled? In order to do good error handling, we have to consider the whole architecture of our application. Let’s say we have an application that periodically updates the UI with the content of some URL.

public static void startTimer() {
	Timer timer = new Timer();
	timer.scheduleAtFixedRate(timerTask(SERVER_URL), 0, 1000);
}
 
private static TimerTask timerTask(final String url) {
	return new TimerTask() {
		@Override
		public void run() {
			try {
				String data = getData(url);
				updateUi(data);
			} catch (Exception e) {
				logger.error("Failed to execute task", e);
			}
		}
	};
}

This is the kind of thinking that we want! Most unexpected errors are unrecoverable, but we don’t want our timer to stop, do we?

What would happen if it did?

First, a common practice is to wrap Java’s (broken) checked exceptions in RuntimeExceptions:

public static String getData(String urlAsString) {
	try {
		URL url = new URL(urlAsString);
		HttpURLConnection connection = (HttpURLConnection) url.openConnection();
 
		// Read the data from the connection
		StringBuilder builder = new StringBuilder();
		try (BufferedReader reader = new BufferedReader(new InputStreamReader(connection.getInputStream()))) {
			String line;
			while ((line = reader.readLine()) != null) {
				builder.append(line);
			}
		}
		return builder.toString();
	} catch (IOException e) {
		throw new RuntimeException(e.getMessage(), e);
	}
}

As a matter of fact, whole libraries have been written with little more value than hiding this ugly feature of the Java language.

Now, we could simplify our timer:

public static void startTimer() {
	Timer timer = new Timer();
	timer.scheduleAtFixedRate(timerTask(SERVER_URL), 0, 1000);
}
 
private static TimerTask timerTask(final String url) {
	return new TimerTask() {
		@Override
		public void run() {
			updateUi(getData(url));
		}
	};
}

If we run this code with an erroneous URL (or the server is down), things go quite bad: We get an error message to standard error and our timer dies.

At this point in time, one thing should be apparent: This code retries whether there’s a bug that causes a NullPointerException or whether a server happens to be down right now.

While the second situation is good, the first one may not be: A bug that causes our code to fail every time will now be puking out error messages in our log. Perhaps we’re better off just killing the timer?

public static void startTimer() // ...
 
public static String getData(String urlAsString) // ...
 
private static TimerTask timerTask(final String url) {
	return new TimerTask() {
		@Override
		public void run() {
			try {
				String data = getData(url);
				updateUi(data);
			} catch (IOException e) {
				logger.error("Failed to execute task", e);
			}
		}
	};
}

Lesson #2: Recovery isn’t always a good thing: You have to consider errors that are caused by the environment, such as a network problems, and what problems are caused by bugs that won’t go away until someone updates the program.

Are you really there?

Let’s say we have WorkOrders which has tasks on them. Each task is performed by some person. We want to collect the people who’re involved in a WorkOrder. You may have come across code like this:

public static Set findWorkers(WorkOrder workOrder) {
	Set people = new HashSet();
 
	Jobs jobs = workOrder.getJobs();
	if (jobs != null) {
		List jobList = jobs.getJobs();
		if (jobList != null) {
			for (Job job : jobList) {
				Contact contact = job.getContact();
				if (contact != null) {
					Email email = contact.getEmail();
					if (email != null) {
						people.add(email.getText());
					}
				}
			}
		}
	}
	return people;
}

In this code, we don’t trust what’s going on much, do we? Let’s say that we were fed some rotten data. In that case, the code would happily chew over the data and return an empty set. We wouldn’t actually detect that the data didn’t adhere to our expectations.

Let’s clean it up:

public static Set findWorkers(WorkOrder workOrder) {
	Set people = new HashSet();
	for (Job job : workOrder.getJobs().getJobs()) {
		people.add(job.getContact().getEmail().getText());
	}
	return people;
}

Whoa! Where did all the code go? All of the sudden, it’s easy to reason about and understand the code again. If there is a problem with the structure of the work order we’re processing, our code will give us a nice crash to tell us!

Null checking is one of the most insidious sources of paranoid programming, and they breed very quickly. Image you got a bug report from production – the code just crashed with a NullPointerException (NullReferenceException for you C#-heads out there) in this code:

public String getCustomerName() {
	return customer.getName();
}

People are stressed! What do you do? Of course, you add another null check:

public String getCustomerName() {
        if (customer == null) return null;
	return customer.getName();
}

You compile the code and ship it. A little later, you get another report: There’s a null pointer exception in the following code:

public String getOrderDescription() {
   return getOrderDate() + " " + getCustomerName().substring(0,10) + "...";
}

And so it begins, the spread of the null checks through the code. Just nip the problem at the beginning and be done with it: Don’t accept nulls.

By the way, if you wonder if we could make the parsing code accepting of null references and still simple, we can. Let’s say that the example with the work order came from an XML file. In that case, my favorite way of solving it would be something like this:

public static Set findWorkers(XmlElement workOrder) {
	Set people = new HashSet();
	for (XmlElement email : workOrder.findRequiredChildren("jobs", "job", "contact", "email")) {
		people.add(email.text());
	}
	return people;
}

Of course, this requires a more decent library than Java has been blessed with so far.

Lesson #3: Null checks hide errors and breeds more null checks.

Conclusion

When trying to be defensive, programmers often end up being paranoid – that is, desperately pounding at the problems where they see them, instead of dealing with the root cause. An offensive strategy of letting your code crash and fixing it at the source will make your code cleaner and less error prone.

Hiding errors lets bugs breed. Blowing up the application in your face forces you to fix the real problem.



Published at DZone with permission of {{ articles[0].authors[0].realName }}, DZone MVB. (source)

Opinions expressed by DZone contributors are their own.

{{ tag }}, {{tag}},

{{ parent.title || parent.header.title}}

{{ parent.tldr }}

{{ parent.urlSource.name }}
{{ parent.authors[0].realName || parent.author}}

{{ parent.authors[0].tagline || parent.tagline }}

{{ parent.views }} ViewsClicks
Tweet

{{parent.nComments}}