Is return boolean for operation success / fail a good practice in Java?

Question

I have some function works with database. I have set a try/catch for error handling here, and display a message, It works fine.

Now the class calling this delete function need to know if there is a error or not. In my case : refresh the GUI if success, nothing to do if fail (as there already show up a message message dialog).

I come up a idea to return boolean in this function.

public static Boolean delete(int id){

    String id2 = Integer.toString(id);
    try {
        String sql = 
                "DELETE FROM toDoItem " +
                "WHERE id = ?;";
        String[] values = {id2};
        SQLiteConnection.start();
        SQLiteConnection.updateWithPara(sql, values);
    } catch (SQLException e) {
        Main.getGui().alert("Fail when doing delete in DataBase.");
        System.out.println("Exception : "+ e.getMessage());
        return false;
    }
    return true;
}

Don't know if this is good or bad, please tell.

---

EDIT :

Here is more detail for How do I use :

Let's say the code above is inside Class A, in Class B :

public boolean deleteItem(int id){
    int i = index.get(id);
    if(theList[i].delete()){  //<---- here is the function from Class A
        theList[i] = null;
        index.remove(id);
        retutn true;
    }
    retutn false; 
}

I need to pass the boolean in more than one class, I don't know if that can better through...

in Class C :

public void toDoList_deleteItem(){
    MyButton btn = (MyButton)source;
    int id = btn.getRefId();
    List toDoList = Main.getToDoList();
    if(toDoList.deleteItem(id)){  //<-------function in Class B
        Main.getGui().refresh();
    }
}

---

Edit 2 :

I have notice the question is somehow more likely asking "What should I handle a Exception at database Layer that affect to GUI Layer ?"... Something like that. Please correct me if the question title should be edit.

Answer 1

It looks like you are returning a boolean status to indicate that an exceptional condition had occurred. Generally, this is not a good practice, for two reasons:

• It encourages an error-prone way of handling exceptions - it is very easy to miss a status check, leading to ignored errors
• It limits your API's ability to report errors - a single pass/fail bit is not always sufficient, it may be desirable to pass more information about the error.

A better approach would be to define an application-specific exception, and use it in your API. This forces the users of your API to pay attention to exceptional situations that may happen, while letting you pass as much (or as little) additional information as you find necessary. At the same time, your code does not get polluted with if (!delete(id)) { /* handle error */ } code on each API call, shrinking your code base, and improving its readability.

Can you tell me more about "define an application-specific exception", or show some code example please?

Here is how I would do it:

public class DataAccessException extends Exception {
    ... // Define getters/setters for passing more info about the problem
}
...
public static void delete(int id) throws DataAccessException {
    try {
        ... // Do something that may lead to SQLException
    } catch (SQLException se) {
        // Do additional logging etc., then
        throw new DataAccessException("Error deleting "+id, se);
    }
}

Note: It is common to give custom exceptions four constructors mirroring the constructors of the Exception class to allow exception chaining. The constructors are described here.

Answer 2

As long as you do not want the caller to know what happens, just that it fails (and that failing is part of its intended behavior) you should be fine.

That being said, I am noticing this: Main.getGui().alert("Fail when doing delete in DataBase.");.

It would seem that you are accessing the GUI layer from some other place. This might cause issues should you decide to multi-thread your application. Also, it is usually considered good practice to have your layers not intersect.

Answer 3

Don't return a Boolean, return a boolean. Since this is not an exception / error condition, it is fine.

Answer 4

Exceptions should be used when you don't expect a failure.

In your case, if it's fine for you that a SQLException is thrown and does not affect your program, it's ok to return a boolean.

If the SQLExcetion causing the delete to fail can cause problems in another part of your application it's better to throw an exception.

Edit:

Based on your edits, it seems that you are doing some maintenance and cleaning when an error happens. In such a case I would recommend to use Exceptions better than using booleans to control the execution.

Answer 5

This question is primarly opinion based. Personally I would prefer not to catch the exception at that point.

Depending on what the caller of delete() should do, you might need other resulutions. So you should better add a throw statement and let the calling method decide if the error is critical - or if it can proceed.

Just true and false is not necessary enough to let the caller decide correctly. He won't know if deletion fails due to database errors, due to foreignkey constraints, or something else.

letting the exception bubble up the call stack will provide the caller with the exact error going on, increasing the chance to handle the error in a proper way, or just displaying a custom error message helping the user to take proper actions.