Archive

Author Archive

Test For Null In Exception Catch Block Indicative Of Anti-Pattern

In a blog posting about Project Coin I came across this example: (first comment is mine)

// Please do not write code like this:
private static void customBufferStreamCopy(File source, File target) {
    InputStream fis = null;
    OutputStream fos = null;
    try {
        fis = new FileInputStream(source);
        fos = new FileOutputStream(target);
        byte[] buf = new byte[8192];
        int i;
        while ((i = fis.read(buf)) != -1) {
            fos.write(buf, 0, i);
        }
    }
    catch (Exception e) {
        e.printStackTrace();
    } finally {
        close(fis);
        close(fos);
    }
}

private static void close(Closeable closable) {
    if (closable != null) {
        try {
            closable.close();
        } catch (IOException e) {
            e.printStackTrace();
        }
    }
}

At first sight and leaving the typos (closable) and other naming issues for what they are, this looks okay. There are however several problems with this code:

  • The code is monolithic.
  • Callers of customBufferStreamCopy have no idea if the call failed or succeeded.
    Since this method is a simple API for copying files, callers should be notified if the copy action failed: the method should throw IOException.
  • Do Not Catch java.lang.Exception. Ever. Please just don’t.
    Never catch Exception directly; catch one of its subclasses. And yes, there are exceptions to this rule, but if you do not know them you should simply adhere to the mantra: Do Not Catch java.lang.Exception.
  • Variable initialization to null.
    This is an ugly hack to prevent compiler errors. Ugly hacks like this are indicative you are doing something suboptimal or even wrong.
  • Test for null in catch block.
    Although I have seen this anti-pattern so many times, I still feel a little sad when I come across an instance of it. If you need to test wether or not something needs cleaning up, your exception handling is needlessly complex. The right way of using objects that need special cleanup or disposal is NOT: 

    // the wrong way: needlessly complex
    
    try {
        // get reference to object
        // use object
    } catch (/* ... */ {
        // error handling
    } finally {
        // dispose of object only if object exists
    }

    It can be simpler:

    // simpler is better (in this case at least)
    
    // get reference to object
    try {
        // use object
    } catch (/* ... */ {
        // error handling
    } finally {
        // dispose of object
    }

Incorporating the above criticisms, we can rewrite the copy method as follows:

// This is better…
private static void customCopy(File source, File target) throws IOException {
    InputStream fis = new FileInputStream(source);
    try {
        OutputStream fos = new FileOutputStream(target);
        try {
            byte[] buf = new byte[8192];
            int i;
            while ((i = fis.read(buf)) != -1) {
                fos.write(buf, 0, i);
            }
        } finally {
            fos.close();
        }
    } finally {
        fis.close();
    }
}

Advantages:

  • No if-statements: simpler execution flow.
  • Better modularity.
  • No compiler hacks.
  • No need for a close utility method.

This code still looks monolithic, but as a direct result of the first two advantages it is now trivial to refactor the method into three separate methods:

// And this is best!
private static void customCopy(File source, File target) throws IOException {
    InputStream fis = new FileInputStream(source);
    try {
        customCopy(fis, target);
    } finally {
        fis.close();
    }
}

private static void customCopy(InputStream source, File target) throws IOException {
    OutputStream fos = new FileOutputStream(target);
    try {
        customCopy(source, fos);
    } finally {
        fos.close();
    }
}

private static void customCopy(InputStream source, OutputStream target) throws IOException {
    byte[] buf = new byte[8192];
    int i;
    while ((i = source.read(buf)) != -1) {
        fos.write(buf, 0, i);
    }
}

Now we have three lean methods that are easy to understand and test. A lot better than the monolithic example we started out with.

Categories: Java Tags: ,

Capitalisation Of Abbreviations In Java Names

HttpURLConnection. Nice class, awful naming: two schemes for abbreviations in one class name. Sun has unfortunately set a bad example which many developers have followed. From looking at this class, we might imply the following rules for capitalization of abbreviations:

  1. if the abbreviation is the first ‘word’ in the name, use initial caps
  2. if the abbreviation is not the first ‘word’ in the name, use all caps

Right. No see if our hypothesis holds for other names. Lets see: AWTError… No luck there. And I found two other gems: javax.rmi.server.UID, javax.sql.RowId. This is somewhat chaotic: it seems we are now down to only one rule:

  1. use capitalization in abbreviations any damn way you like

Do not get me wrong, I like having as little rules as possible, but this rule – or rather absence of any rules – makes life unnecessary difficult in certain corner cases which I will describe below.

But the Sun Code Conventions…?
In the Sun Java Code Conventions document (ch. 9) you can only find one reference to abbreviations, and only for class names: Use whole words—avoid acronyms and abbreviations (unless the abbreviation is much more widely used than the long form, such as URL or HTML). That is it. The funny thing is that in this document, Sun does not follow the informal rule of using caps in (American) English: when abbreviating words that are originally spelled with lower case letters, there is no need for capitalization. So their example should have been: … such as url or html. If Sun had done so, there might have been no need for this article.

New rules
Right now I would like to dive head first and postulate a new set of rules for how to capitalize abbreviations in Java names:

  1. treat an abbreviation as a word

That is it, that is all of it. Before you accept that this is all of it, look at what ‘normal’ words and abbreviations have in common. Both:

  • are a character sequence comprised of letters
  • are used for communication
  • have a specific meaning (within a certain context; human language is ambiguous)
  • should not be altered: you cannot add or remove letters without risk of altering what you are trying to communicate

And for the differences between the two:

  1. an abbreviation is comprised of multiple words

Let us look at it from a different angle: an Abbreviation class can easily be modeled as a subclass of Word where the Abbreviation class has a derivedFrom property in the form of a List of Word instances.

Also, please note that acronyms (like ‘radar’ and ‘laser’) are already considered to be words and not “merely” abbreviations.

Using abbreviations as words should come quite natural: the first two rules of this article only used single quotes to imply such a relation and I think very few readers did not understand their meaning.

So an abbreviation is a word: now what?
Now that we have established that an abbreviation “is a” word, it follows that we should apply the same rules we have for capitalization on any type of word. We do not need any new rules, just the old rules! Following this logic the earlier examples should actually be: HttpUrlConnection, AwtError, Uid, RowId.

This automatically fixes a few problems we might otherwise face:

  • property names and variable names derived from class names look ‘good’
    If a class has exactly one property of a certain type, we are likely to name that property based on that type. If for instance we have a class Uuid and a person has precisely one uuid, the Person class would likely have a getUuid() method which by definition of the JavaBeans api would result in a uuid property. Compare that to getUUID() and an equivalent uUID property.
  • we know where one abbreviation ends and another begins
    HttpsId clearly differs from HttpSid. HTTPSID introduces unnecessary ambiguity and is far less readable.
  • code generation tools have an easier time
    If for instance you would like your IDE to generate database tables from your persistable classes – or vice versa -, there is no need for manual renaming of identifiers.
  • do not shout
    Call me old fashioned, but I grew up in the days where netiquette was in high esteem on the internet. In the netiquette, use of all caps is described as the written equivalent of shouting. As shouting hurts my ears, “text shouting” hurts my visual cortex.

In a nutshell
Conclusion: accept that an abbreviation is a word, and you do not need any new rules for capitalization. As added benefits readability of your code is improved and tools need less manual handling.

Categories: Java Tags:
Follow

Get every new post delivered to your Inbox.