Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I've seen this sort of pattern in code before:

//pseudo C# code

var exInfo = null;  //Line A

try
{
  var p = SomeProperty;         //Line B
  exInfo = new ExceptionMessage("The property was " + p);           //Line C
}
catch(Exception ex)
{
  exInfo.SomeOtherProperty = SomeOtherValue;  //Line D
}

Usually the code is structured in this fashion because exInfo has to be visible outside of the try clause. The problem is that if an exception occurs on Line B, then exInfo will be null at Line D. The issue arises when something happens on Line B that must occur before exInfo is constructed. But if I set exInfo to a new Object at line A then memory may get leaked at Line C (due to "new"-ing the object there). Is there a better pattern for handling this sort of code? Is there a name for this sort of initialization pattern?

By the way I know I could check for exInfo == null before line D but that seems a bit clumsy and I'm looking for a better approach.

share|improve this question
    
This code looks like a faulty strange code. –  Ubiquité Oct 17 '12 at 20:13
    
What is a "faulty strange code"? I did remove some of the context to try to pare down to the essence of the question but I assure you this is not a contrived example. –  Onorio Catenacci Oct 17 '12 at 20:54
    
It's an error to use exInfo without being sure it's not null. Of course this is a bad pattern, there is no argument possible on this. –  Ubiquité Oct 17 '12 at 21:13
    
Seems to me like this question is a classic case of attempting to overdesign. –  Dunk Oct 17 '12 at 22:50
    
@Dunk would you mind elaborating on your comment? I fail to see how this is overdesigning anything. It's existing code and I've seen the pattern in other places; I was just trying to find a more general solution to the problem. –  Onorio Catenacci Oct 19 '12 at 15:41

4 Answers 4

up vote 5 down vote accepted

You should initialize exInfo before the try block. In .NET you won't leak any memory at Line C.

I'm going to try and make a concrete example out of your pseudocode, and see if that helps. Assume db.getList() runs some SQL and returns NULL if nothing found, a list if elements were found, and throws an exception on error.

List<String> stuff = new ArrayList<String>();

try {
   stuff = db.getList("SELECT someColumn FROM someTable");
}
catch (SqlException e) {
   log.error("Couldn't retrieve stuff!", e);
}

return stuff;

In Java and other garbage-collected languages, no memory would be leaked after your line C if you initialized the variable previously. The initial object would have no references associated with it and would be then eligible for garbage collection. This pattern allows you to keep code calling this code null-safe, since it will always return a valid List<>.

share|improve this answer
    
Thanks--this is exactly the sort of information I was seeking. –  Onorio Catenacci Oct 17 '12 at 20:06
    
So how do you know that the query failed versus there was no matching data using this approach? –  Dunk Oct 17 '12 at 22:54
    
@Dunk In many cases, I don't think you want a SQL error to crash the application. In our case, the most likely explanation for SQL failing in production is that the DB connection went down, and monitoring would detect that. We also monitor the ERROR level logging. So, fail soft (still logging the exception so we can see precisely what happened) and return an empty list so that upstream code doesn't need to null-check. –  Michael K Oct 18 '12 at 12:34
    
And you don't have to handle the error as I did here...the general pattern is only to prevent returning a null list. You may need a different contract that allows null in case of errors. In that case this pattern wouldn't be useful. –  Michael K Oct 18 '12 at 12:36

It's a good idea for a variable to never contain an incorrect value, if at all possible. That way there's no possible way, for example, for maintainers to later insert a bunch of statements between Line A and the try block, then somewhere in there accidentally use exInfo when it's null. The general solution when you're tempted to initialize a value to null due to scope reasons is to break off the initialization code into its own function, so you would call:

var exInfo = createExInfo();

And createExInfo would contain:

try
{
  var p = SomeProperty;
  return new ExceptionMessage("The property was " + p);
}
catch(Exception ex)
{
  var exInfo = new ExceptionMessage();
  exInfo.SomeOtherProperty = SomeOtherValue;
  return exInfo;
}
share|improve this answer
    
Excellent suggestions Karl. –  Onorio Catenacci Oct 18 '12 at 13:16

I would refactor like this:

        var p = null;
        var exInfo = null;

        try
        {
            p = SomeProperty;         //Line B
        }
        catch (Exception ex)
        {
            exInfo = new ExceptionMessage("The property was " + p == null ? "foo" : "bar");           //Line C
            exInfo.SomeOtherProperty = SomeOtherValue;  //Line D
        }

        exInfo.BlahBlahBlah = "Blah";
share|improve this answer

How about creating class that has an event handler? Then initialize exInfo before initializing var p, which would guarantee that exInfo would not be null?

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.