Delete Entry from SQL-Table and confirm the deletion to a CommandSender in a spigot plugin

Question

I'm working on a spigot plugin, that deals with messages to from players on a server to the admin's. It's the same plugin this question relates to: SQL structure to save coordinates for spigot plugin . I've got a command, which allowes access to the inbox, and does all kinds of operations on the data. One of these operations is deleting an entry by it's ID.

I've got a simple SQL query to delete it, and want to display a message, depending on if the message could be deleted or not. Therefore I wrote this method in my Command-Class:

private void deleteMessage(CommandSender sender, int messageId) {
    int deleted = DatabaseMethods.deleteMessage(messageId);
        
    if (deleted == 0) {
        sender.sendMessage(MessageManager.couldNotBeFound(messageId));
    } else {
        sender.sendMessage(MessageManager.deletedMessage(messageId));
    }
}

and this is the method that accesses the database:

public static int deleteMessage(int messageId) {
    try (final PreparedStatement ps = BasePluginAPI.getDatabaseConnection()
            .prepareStatement("delete from MELDUNG where ID = ?")) {
        ps.setInt(1, messageId);
        return ps.executeUpdate();
    } catch (SQLException e) {
        e.printStackTrace();
        return 0;
    }
}

I don't know if this is the best way to approach it. I have two main questions:

1. Is an early return better for the first method? Or is it just a question of personal taste?
2. Can the SQL-handling be improved? Should maybe the method return a boolean instead of an int?

Besides these specific questions, of course general feedback is very welcome as well! :)

Answer 1

Regarding your questions:

1. Is an early return better for the first method? Or is it just a question of personal taste?

The alternative you suggest would be:

private void deleteMessage(CommandSender sender, int messageId) {
    int deleted = DatabaseMethods.deleteMessage(messageId);
    if (deleted == 0) {
        sender.sendMessage(MessageManager.couldNotBeFound(messageId));
        return;
    } 
    sender.sendMessage(MessageManager.deletedMessage(messageId));
}

I generally don't recommend it. A developer might delete the empty return thinking it's unnecessary causing a bug. The if-else is more readable in my opinion.

2. Can the SQL-handling be improved? Should maybe the method return a boolean instead of an int?

If the delete operation can return only 0 or 1, then it should be a boolean. However, there are more than two outcomes: the database is down, the database call times out, etc. "Message not found" is just one of the reasons for deleteMessage to return 0. So instead of MessageManager.couldNotBeFound(messageId)), would be more appropriate to say "message cannot be deleted".

To tell the user "message not found", you can let the second method throw the exception and catch it in the first method.

One more thing, from the method signature is not clear why deleteMessage(CommandSender sender, int messageId) needs a CommandSender, a better name can be deleteMessageAndNotify.