Offensive programming: paranoid, offensive, offensive or defenseless programming

How to make your code concise and obedient at the same time?

Have you ever met an application that behaved obviously strange? Well, you know, you push a button and nothing happens. Or the screen suddenly turns black. Or the application falls into a “weird state” and you have to restart it in order for it to work again.

If you had this experience, then you probably became a victim of a certain form of defensive programming , which I would like to call "paranoid programming." Defender cautious and prudent. Paranoid feels fear and acts strangely. In this article I will propose an alternative approach: Offensive programming .
')

Watchful reader

What does paranoid programming look like? Here is a typical java example

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 the URL as a string. An unexpected amount of code to perform a very simple task, but this is Java.

What is wrong with this code? The code seems to cope with all the possible errors that may occur, but it does it in a terrible way: just ignoring them and continuing to work. This practice is unconditionally supported by Java's checked exceptions (an absolutely bad invention), but similar behavior is seen in other languages.

What happens if an error occurs:

• If the URL passed is invalid (for example, “http // ..” instead of “http: // ...”), the next line will produce a NullPointerException: connection = (HttpURLConnection) url.openConnection (); At the moment, the unfortunate developer who gets the error message has lost the whole context of the real error and we don’t even know which URL caused the problem.
  
  
• If the web site in question does not exist, the situation becomes much, much worse: the method will return an empty string. Why? Result StringBuilder builder = new StringBuilder (); will still be returned from the method.

Some developers claim that such code is good, since our application will not fall. I would argue that there are worse things that can happen to our application besides falling. In this case, the error will simply cause the wrong behavior without any explanation. For example, the screen may remain blank, but the application does not give an error.

Let's look at the code written by the defenseless method.

 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 (required in Java, but not in other languages ​​I know) shows that this method may not work and the caller must be ready to deal with it.

This code is more concise and if an error occurs, the user and the logger will (presumably) get the correct error message.

Lesson 1 : Do not handle exceptions locally.

Barrier flow

Then how should errors of this kind be handled? In order to do error handling well, we need to consider the entire architecture of our application. Let's assume that we have an application that periodically updates the UI with the contents of a 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 exactly the type of thinking we want! Of the most unforeseen errors, there is no way to recover, but we don’t want our timer to stop this, is it?

And what happens if we want it? First, there is the well-known practice of wrapping Java's 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); } } 

In fact, the libraries were written with a little more meaning than hiding this ugly Java language feature.

Now we can 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 invalid URL (or the server is not available), everything will become bad enough: We will receive an error message to the standard error output stream and our timer will die.

At this point in time, one thing should be obvious: This code repeats actions, regardless of whether it is a bug that causes a NullPointerException, or just the server is not available right now.

While the second situation suits us, the first may not be very: the bug that causes our code to fall every time will now litter in our error log. Maybe we'd better just kill 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 is not always good. You should take into account errors caused by the environment, such as network problems, and errors caused by bugs that will not disappear until someone corrects the code.

Are you really there?

Let's assume that we have a WorkOrders class with tasks. Each task is implemented by someone. We want to gather people who are involved in WorkOder . You have probably met with similar code:

 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 really trust what is happening, do we? Let's assume that we were fed bad data. In this case, the code will happily swallow the data and return an empty set. We do not really notice that the data does not meet our expectations.

Let's clean the code:

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

Hey! Where did all the code go? Suddenly it is again easy to discuss and understand the code. And if there is a problem with the structure of the task being processed, our code will happily break down and inform us about it.

Checking for null is one of the most insidious sources of paranoid programming and their number is rapidly increasing. Imagine that you got a bug report from production - the code just fell from the NullPointerException in this code.

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

People under stress! And what to do? Of course, you added another check to null.

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

You compile the code and upload it to the server. After a while, you get another report: null pointer exception in the following code.

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

And this is how it begins - the propagation of null checks throughout the code. Just stop the problem at the very beginning: do not take nulls.

And by the way, if you are interested in whether we can force the parser code to accept nulls and stay simple, then yes, we can. Suppose that in the example with tasks we get data from an XML file. In this case, my favorite way to solve this problem would be:

 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 the one that is in Java now.

Lesson 3 : Null checks hide errors and generate more null checks.

Conclusion

Trying to write a security code, programmers often end up being paranoid — desperately snapping at all the problems they see, instead of dealing with the root cause. The defenseless strategy of allowing the code to fall and correcting the source of the problem will make your code cleaner and less error prone.

Hiding mistakes leads to the multiplication of bugs. The explosion of the application, right in front of your nose, makes you solve a real problem.

---

For the translation, thanks to Olga Chernopitskaya and the Edison company, which is developing an internal personnel testing system and an application for recording staff time .