Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I wanted to get the boolean status from a function. Here is my sample code, wherein I used return in try and catch, to get the status of function.

public static void main(String[] args) {
        System.out.println(getState() ? "successful" : "failed");
}

public static boolean getState() {
        SAXBuilder builder = new SAXBuilder();
        File xmlFile = new File("tile.xml");
        Set<String> set = new TreeSet<String>();
        try {
            Document document = (Document) builder.build(xmlFile);
            Element rootNode = document.getRootElement();
            System.out.println(rootNode);
            return true;
        } catch (IOException io) {
            System.out.println(io.getMessage());
            return false;
        } catch (JDOMException jdomex) {
            System.out.println(jdomex.getMessage());
            return false;
        }
    }

Does it makes sense ? Can I improve it ? Assuming, that there might be more than 10 catch blocks, then should I use 10 return statements, in each catch block.

share|improve this question
1  
Which Java version are you using? With Java7 multicatch is available and ideal when you have identical catch blocks but don't want to widen your catch scope. See details at docs.oracle.com/javase/7/docs/technotes/guides/language/… –  Dan Midwood Dec 3 '12 at 17:22
    
Dan Nidwood : I'm using java 1.6 –  srk Dec 4 '12 at 4:58

2 Answers 2

up vote 5 down vote accepted

For only 2 or 3 instances, I may use multiple return statements, but only in the case of if-else chains or switches. Returns within a try catch block can be done, but also can become complicated when you start mixing in finally with it. In the case of try-catch blocks or several possible 'return' paths, I use a variable and set it as necessary, and return it at the end. If you want an early return to bypass some code, I tend to take that code and wrap it up in a separate method, and use an if statement with my value to return to determine whether to execute that method.

public static void main(String[] args) {
        System.out.println(getState() ? "successful" : "failed");
}

public static boolean getState() {
        boolean success = false;
        SAXBuilder builder = new SAXBuilder();
        File xmlFile = new File("tile.xml");
        Set<String> set = new TreeSet<String>();
        try {
            Document document = (Document) builder.build(xmlFile);
            Element rootNode = document.getRootElement();
            System.out.println(rootNode);
            success = true;
        } catch (IOException io) {
            System.out.println(io.getMessage());
        } catch (JDOMException jdomex) {
            System.out.println(jdomex.getMessage());
        }

        return success;
}
share|improve this answer

I agree, that you should avoid return in try/catch/finally blocks, mostly because it is not always for everyone clear what is happening there. This goes hand in hand with the pattern that exceptions should not be used as flow control statements.

Depending on the use case, one different suggestion compared to Drake Clarris is to extract the try catch outside the normal logic. This makes the source code more readable and more clear.

public static void main(String[] args) {
        System.out.println(getState() ? "successful" : "failed");
}

public static boolean getState() {
    final File xmlFile = new File("tile.xml");
    final Element rootNode = getRoodNode(xmlFile);
    if (rootNode == null)
        return false;
    System.out.println(rootNode);
    return true
}

private static Element getRoodNode(final File xmlFile)
{
    Element rootNode = null;
    try {
        final Document document = (Document) new SAXBuilder().build(xmlFile);
        rootNode = document.getRootElement();
    } catch (IOException io) {
        System.out.println(io.getMessage());
    } catch (JDOMException jdomex) {
        System.out.println(jdomex.getMessage());
    }
    return rootNode;
}
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.