Sean has a lucrative career as a consultant/contractor. As such, he spends a great deal of time in other people’s code bases, and finds things like a method with this signature:
public boolean isTableEmpty()
Already, you’re in trouble. Methods which operate directly on “tables” are a code-smell, yes, even in a data-driven application. You want to operate on business objects, and unless you’re a furniture store, tables are not business objects. You might think in those terms when building some of your lower-level components, but then you’d expect to see things like string tableName
in the parameter list.
Now, maybe I’m just being opinionated. Maybe there’s a perfectly valid reason to build a method like this that I can’t imagine. Well, let’s check the implementation.
public boolean isTableEmpty()
{
boolean res = false;
Connection conn = cpInstance.getConnection();
try (PreparedStatement ps = conn.prepareStatement("select * from some_table")) {
try (ResultSet rs = ps.executeQuery()) {
if (rs.first()) {
res = true;
}
}
catch (SQLException e) {
e.printStackTrace();
} finally {
try {
conn.close();
} catch (SQLException e) {
e.printStackTrace();
}
}
return res;
}
Even if you think this method should exist, it shouldn’t exist like this. No COUNT(*)
or LIMIT
in the query. Using exceptions as flow control. And the best part: returning the opposite of what the method name implies. false
tells us the table is empty.