Java and SQL : return null or throw exception?

Question

This is another debated subject, but this time i am searching only for the simple and documented answers. The scenario :

Let's assume the following method:

 public static Hashtable<Long, Dog> getSomeDogs(String colName, String colValue) {
  Hashtable<Long, Dog> result = new Hashtable<Long, Dog>();
  StringBuffer sql = null;
  Dog dog = null;
  ResultSet rs = null;
      try {
          sql = new StringBuffer();
          sql.append("SELECT * FROM ").append("dogs_table");
          sql.append(" WHERE ").append(colName).append("='");
          sql.append(colValue).append("'");
          rs = executeQuery(sql.toString());
              while (rs.next()) {
                  dog= new Dog();
                  //...initialize the dog from the current resultSet row
              result.put(new Long(dog.getId()), dog);
              }
          }
     catch (Exception e) {
         createErrorMsg(e);
         result = null; //i wonder....
         }
     finally {
         closeResultSet(rs); //this method tests for null rs and other stuff when closing the rs.
     }
   return result;
 }

Questions :

1. What ways do u suggest on improving this technique of returning some dogs, with some attribute?

2. rs.next() will return false for a null ResultSet, or will generate an exception, like this one :

String str = null; System.out.println(str.toString());

3. What if, while initializing a dog object from the current row of the ResultSet, something bad happens, like : connection failure, incompatible value has been passed to dog property setter, etc? I might have 10 elements in the hashtable by now, or none (first row). What will be the next action : a) return null hashtable; b) return the result hashtable, the way it is at this stage; c) throw an exception : what will be the exception type here?

4. I think u will all agree on this one : nothing bad happens, there are no rows on the interrogation, a null value will be return. However, @Thorbjørn Ravn Andersen said here that i should return a NullObject instead of a null value. I wonder what that is.

5. I've noticed people and groups of people saying that one should split the application into layers, or levels of some kind. Considering the example above, what layers are here, except these ones i can think of :

Layer1 :: Database layer, where actions are performed : this method.

Layer2 :: ??? : some layer where the new Dog object is constructed : my Dog object.

Layer3 :: ? : some layer where i intend to do something with the collection of dogs : GUI layer, mostly, or a sub-layer of user interface.

Following the application flow, if an exception occures in the 1st layer, what is the best think to do with it? My thoughts : catch the exception, log the exception, return some value. Is that the best practice?

Manny thanks for your answers, i look forward to see what other people think of these matters.

Answer 1

I'd avoid the following

   sql.append("SELECT * FROM ").append("dogs_table");
   sql.append(" WHERE ").append(colName).append("='");
                        sql.append(colValue).append("'");

and instead use a PreparedStatement with its associated parameter setter methods (setString()) etc. This will prevent problems with values for colValue having quotes, and SQL injection attacks (or more generally, colValue forming some SQL syntax).

I would never return a null if the collection was merely empty. That seems very counter-intuitive, and completely unexpected from the client's point of view.

I wouldn't recommend returning a null in error conditions, since your client has to explicitly check for this (and will probably forget). I would return an empty collection if need be (this may be analogous to your comment re. a null object), or more likely throw an exception (depending on the circumstances and the severity). The exception is useful in that it will carry some information relating to the error encountered. Null tells you nothing.

What should you do if encountering a problem whilst building a Dog object ? I think that depends on how robust and resilient you want your application to be. Is it a problem to return a subset of Dogs, or would it be completely catastrophic and you need to report this ? That's an application requirement (I've had to cater for either scenario in the past - best-effort or all-or-nothing).

A couple of observations. I'd use HashMap rather than the old Hashtable (synchronised for all access and , more importantly, not a proper Collection - if you have a Collection you can pass it to any other method expecting any Collection), and StringBuilder over StringBuffer for similar reasons. Not a major problem, but worth knowing.

Answer 2

You ask five questions

1. What ways do u suggest on improving this technique of returning some dogs, with some attribute?

Several, actually.

• Your method is static - this isn't terrible, but leads to you using another static "executeQuery", which smells of Singleton to me...
• The class "Dogs" violates an OO naming practice - plural nouns don't make good class names unless one instance of the class holds a collection of things - and it appears that Dogs is actually "Dog".
• HashTable is all but deprecated. HashMap or ConcurrentHashMap give better performance.
• I can't see a reason to create the first part of your query with multiple appends - it's not bad, but it's less readable than it might be, so sql.append ("SELECT * FROM dogs_table WHERE "); makes a more sensible beginning if you're just going to hardcode the selected columns (*) and table name (dogs_table) anyway.

2. rs.next() will return false for a null ResultSet, or will generate an exception

This doesn't seem to be a question, but yes, rs.next() returns false as soon as there's no longer any rows to process.

3. What if, while initializing a dog object from the current row of the ResultSet, something bad happens

If "something bad happens", what you do next is up to you and your design. There's the forgiving approach (return all the rows you can), and the unforgiving (throw an exception). I tend to lean towards the "unforgiving" approach, since with the "forgiving" approach, users won't know that you haven't returned all the rows that exist - just all the ones you'd gotten before the error. But there might be cases for the forgiving approach.

4. I think u will all agree on this one : nothing bad happens, there are no rows on the interrogation, a null value will be return.

This isn't something on which there's an obvious right answer. First, it's not what's happening in the method as written. It's going to return an empty HashTable (this is what was meant by a "null object"). Second, null isn't always the answer in the "no results found" case.

I've seen null, but I've also seen an empty result variable instead. I claim they're both correct approaches, but I prefer the empty result variable. However, it's always best to be consistent, so pick a method of returning "no results" and stick with it.

5. I've noticed people and groups of people saying that one should split the application into layers, or levels of some kind.

This is harder to answer than the others, without seeing the rest of your application.