HiveBrain v1.2.0
Get Started
← Back to all entries
debugjavaMajor

Using finally with return statement or not

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
statementwithreturnusingfinallynot

Problem

I was wondering if it is the proper way to always put the return statement of a function with a try-catch clause in the finally clause.

I have for example this function which returns the version of an sql server:

public String getVersion() {
        String version = new String();
        final String query = "Select VERSION()";
        Connection con = null;
        Statement stmt = null;

        try {
            Helper.println("Getting driver...");
            Class.forName(_driver);

            Helper.println("Connecting to database...");
            con = DriverManager.getConnection(_url, _user._username, _user._password);
            stmt = con.createStatement();

            Helper.println("Executing query...");
            ResultSet rs = stmt.executeQuery(query);
            rs.next();
            version = rs.getString(1);
        } catch (SQLException se) {
            Helper.printErrln("SQL Error: " + se.getErrorCode() + ' ' + se.getMessage());
        } catch (ClassNotFoundException e) {
            Helper.printErrln("Driver Error: " + e.getMessage());
        } finally {
            try {
                if (stmt != null) {
                    stmt.close();
                }
            } catch (SQLException se) {
            }
            try {
                if (con != null) {
                    con.close();
                }
            } catch (SQLException se) {
            }  
            return version;
        }
    }


I return the version variable always in the finally clause in situations like these and it particularly convenient when I need to return a list because if something went wrong then I will just be left with an empty list which I prefer. For example:

```
public List getDatabases() {
List databases = new ArrayList<>();
Connection con = null;
Statement stmt = null;

try {
Helper.println("Getting driver...");
Class.forName(_driver);

Helper.println

Solution

Return statement in the finally block is almost always a very bad idea.....

The sematics of the finally block are complicated, but, if there is a return statement the try block, or in a catch block, then those will be called, and then the finally block will run, and it will change the return value.

IDE's, and the compiler, will complain about this:

Code like the following, will return 2 .... always:

private static int doSomething() {
    try {
        .... something here....
        return 0;
    } catch (Exception e) {
        return 1;
    } finally {
        return 2;
    }

}


The point is that it is very bad practice to have returns in the finally block. Don't.....

EDIT: Update to include a better solution.... Use Java try-with-resources (since Java7).

private static final String VERSION_QUERY = "Select VERSION()";
private static final String VERSION_NOT_FOUND = "";

public String getVersion() {

    try {
        Helper.println("Getting driver...");
        Class.forName(_driver);
    } catch (ClassNotFoundException e) {
        Helper.printErrln("Driver Error: " + e.getMessage());
        return VERSION_NOT_FOUND;
    }

    Helper.println("Connecting to database...");
    try (Connection con = DriverManager.getConnection(_url, _user._username, _user._password);
         Statement stmt = con.createStatement();) {

        Helper.println("Executing query...");
        try (ResultSet rs = stmt.executeQuery(VERSION_QUERY);) {
            rs.next();
            return rs.getString(1);
        }
    } catch (SQLException se) {
        Helper.printErrln("SQL Error: " + se.getErrorCode() + ' ' + se.getMessage());
        return VERSION_NOT_FOUND;
    }
}

Code Snippets

private static int doSomething() {
    try {
        .... something here....
        return 0;
    } catch (Exception e) {
        return 1;
    } finally {
        return 2;
    }

}
private static final String VERSION_QUERY = "Select VERSION()";
private static final String VERSION_NOT_FOUND = "";

public String getVersion() {

    try {
        Helper.println("Getting driver...");
        Class.forName(_driver);
    } catch (ClassNotFoundException e) {
        Helper.printErrln("Driver Error: " + e.getMessage());
        return VERSION_NOT_FOUND;
    }


    Helper.println("Connecting to database...");
    try (Connection con = DriverManager.getConnection(_url, _user._username, _user._password);
         Statement stmt = con.createStatement();) {

        Helper.println("Executing query...");
        try (ResultSet rs = stmt.executeQuery(VERSION_QUERY);) {
            rs.next();
            return rs.getString(1);
        }
    } catch (SQLException se) {
        Helper.printErrln("SQL Error: " + se.getErrorCode() + ' ' + se.getMessage());
        return VERSION_NOT_FOUND;
    }
}

Context

StackExchange Code Review Q#51509, answer score: 20

Revisions (0)

No revisions yet.