Fog Creek Software
Discussion Board




Exception handling: where to put initialization

Easy in C++: RAII pattern, right?

In C# or Java we have two alternatives:

ItemContext ctx = null;
try
{
  ctx = ItemContext.Open (@"\machine\Legal Documents");

  [...]
}
finally
{
  if (ctx != null) ctx.Dispose();
}

..or..

ItemContext ctx = ItemContext.Open (@"\machine\Legal Documents");
try
{
  [...]
}
finally
{
  ctx.Dispose();
}

Which one do you prefer? Why?

Me
Sunday, February 01, 2004

I must be missing something because I'd obviously prefer the first because it actually catches the error!

With the second you're potentially going to end up with an unhandled exception (unless you're proposing the method that called this one will have a try..catch block in it.

I'd still go for the first and catch the error at the most specific level where I can probably give some more meaningful diagnostics.

Gwyn
Sunday, February 01, 2004

With the second you're potentially going to end up with an unhandled exception (unless you're proposing the method that called this one will have a try..catch block in it.

??? neither version catches the error, presumably the intention is that it's caught further up the call stack.

I'd have a slight preference for the second variant, but better than either is the using statement:

using (ctx = ItemContext.Open (@"\machine\Legal Documents")
{
  [...]
}

Joe
Sunday, February 01, 2004

I concur with Joe.  'using' is the way to go.

SomeBody
Sunday, February 01, 2004

Sorry! See I knew I must have missed something! Glossed over the absence of the Catch statement!

Rereading it I can't really find a preference. The second approach (or its using variant) looks attractive but surely it depends... it must depend on the number of different objects I create within that method, espeically if they have different lifetimes within the method, e.g.

Create A
Create B
Dispose A
Create C
Dispose B
Dispose C

In this case you're really have to go for the first option and in your finally block do a possible free of all 3.

Of course the way I manage my objects could be non-standard (I am my own mentor and guide on these sorts of things!) and perhaps it is more normal to wait until the end before disposing:

Create A
Create B
Create C
Dispose A,B,C

In fact, I really should go and look up a bit more detail about disposing in .Net as I'm still a bit ignorant about all the implications.

Gwyn
Sunday, February 01, 2004

I consider the second to be absolutely the proper Java idiom for ensuring release of a resource.  Obviously, neither is outright wrong.  The first example contains pointless activity and clutter, and is therefore inferior.

However, the first one has one minor "advantage" in that nested allocations of resources can be cleaned up, albeit somewhat carelessly, in a single finally.  I see that done all the time, and I don't usually give the programmer any grief over it.  With JDBC, you often must nest releases three deep.  (Connection, Statement, ResultSet.)  Doing all these close calls in a single finally, using the null check of example 1, tends to work, as none of the JDBC close methods seem to ever throw exceptions in practice.  (It's still not truly correct though, and is can be burned by an unusual but properly behaving driver.)

I've found that average Java programmers get really unconmfortable around the nested finally for some reason.  In fact, merely having try without a catch seems to often baffle them.  Still, I'd rather the second example by far.  If you make an exception to the typical Java coding-style rules to plop the release all on a line, it also reads quite clean once you're familiar with it.

  Connection connection = dataSource.getConnection();
  try
  {
    Statement statement = connection....
    try
    {
      // do work
    } finally { statement.close(); }
  } finally { connection.close(); }

veal
Sunday, February 01, 2004

As veal says, the second approach seems a lot more natural in Java. If you're unable to retrieve a resource, you can't clean it up.

Julian
Monday, February 02, 2004

I do (in C#)

Connection con = null;
try
{
  con = new Connection();  // Actually a static DataLayer.GetConnection() here
  con.Open();
  Command cmd = con.CreateCommand();
  DataReader dr = cmd.ExecuteReader();
  whild (dr.Read())
  {
    // do work
  }
  dr.Close();
  // cmd goes out of scope, there is no cmd.Close() method
}
finally
{
  if (null != con) con.Close();
}

In .NET, calling Close() on a closed connection does not raise an exception.

Richard P
Monday, February 02, 2004

I'm kinda bored, so let's look closer...
(I'll borrow the C# version of approach 1 from Richard P)

1  Connection con = null;
2  try {
3   con = new Connection();
4    // do work
5  } finally {
6   if (null != con)
7      con.Close();
8  }

Well, when line 3 fails, con remains null.  So for a failure of the assignment, that finally block becomes superfluous since we know con would be null and the conditional call to close would be skipped.  So we really have no reason to place the assignment within the try, since we'd have no reason to want this finally to run if the assignment fails.

1  Connection con = null;
2  con = new Connection();
3  try {
4    // do work
5  } finally {
6   if (null != con)
7      con.Close();
8  }

Ok, so now we can see that the assignment of null is an utterly useless activity in our revised but functionally equivalent snippet, since the very next line either obliterates that work or else skips the remainder of the snippet.

1  Connection con = new Connection();
2  try {
3    // do work
4  } finally {
5   if (null != con)
6      con.Close();
7  }

But wait... now con could *never* be null, so the condition degenerates to "if true".  How silly would we be to keep that!

1  Connection con = new Connection();
2  try {
3    // do work
4  } finally {
5      con.Close();
6  }

Ahh. Clean and functionally identical.

veal
Monday, February 02, 2004

*  Recent Topics

*  Fog Creek Home