views:

118

answers:

6

If I am using a try/catch/finally block where and how should I initialize variables? For example say I'm trying to use a FileStream . I want to catch any exceptions thrown while creating or using the stream. Then regardless of whether there were any problems or not I want to ensure any stream created is closed.

So I'd do something like this:

        System.IO.FileStream fs;
        try
        {
            fs = new System.IO.FileStream("C:\test.txt", System.IO.FileMode.Open);
            //do something with the file stream
        }
        catch (Exception exp)
        {
            //handle exceptions
        }
        finally
        {
            //ERROR: "unassigned local variable fs"
            if (fs != null)
            {
                fs.Close();
            }
        }

However this gives me an error in the finally block saying unassigned local variable fs. Yet, if I change the declaration of fs to System.IO.FileStream fs = null it works.

Why do I need to explicitly set fs to null? I've also tried declaring fs in the try block, but then I get the error The name fs does not exsist in the current context in finally block.

BTW: I know I could use a Using block, but the point of my question is to understand the correct usage of a try/catch/finally block.

+1  A: 

You are close. I'd set the declaration to be null.

System.IO.FileStream fs = null;
try
{
    fs = new System.IO.FileStream("C:\test.txt", System.IO.FileMode.Open);
    //do something with the file stream
}
catch (Exception exp)
{
    //handle exceptions
}
finally
{
    //ERROR: "unassigned local variable fs"
    if (fs != null)
    {
        fs.Close();
    }
}

I think that is very acceptable when you can't use a using statement.

Jerod Houghtelling
+3  A: 

Assign fs = null;

  System.IO.FileStream fs = null;
    try
    {
        fs = new System.IO.FileStream("C:\test.txt", System.IO.FileMode.Open);
        //do something with the file stream
    }
    catch (Exception exp)
    {
        //handle exceptions
    }
    finally
    {
        //ERROR: "unassigned local variable fs"
        if (fs != null)
        {
            fs.Close();
        }
    }
Michael Bazos
He asked 'Why do I need to explicitly set fs to null?'
BioBuckyBall
+2  A: 

The C# design team feels that making sure you explicitly initialize things is a good idea. I tend to agree; I've been bitten by bugs from uninitialized variables enough in C++.

BioBuckyBall
+1  A: 

Initializing fs to null is the correct usage. The compiler wants to ensure that you are reading only initialized variables, to avoid serious mistakes. And it cannot garantee that the initialization in your try block is ever executed

Hbas
+4  A: 

See section 5.3 of the specification.

http://msdn.microsoft.com/en-us/library/aa691172(VS.71).aspx

At a given location in the executable code of a function member, a variable is said to be definitely assigned if the compiler can prove, by static flow analysis, that the variable has been automatically initialized or has been the target of at least one assignment.

With your try/catch/finally, the assignment of the try block cannot be guaranteed when you attempt to access the object in the finally block. As you have seen, you can meet the requirement by assigning an initial value to the variable (null, in this case).

Anthony Pegram
A: 

The purist in me would want to do something like this:

    void DoSomethingWithStream()
    {
        try
        {
            System.IO.FileStream fs = new System.IO.FileStream(@"C:\test.txt", System.IO.FileMode.Open);
            try
            {
                // do something with the file stream

            }
            catch (Exception ex)
            {
                // handle exceptions caused by reading the stream,
                // if these need to be handled separately from exceptions caused by opening the stream
            }
            finally
            {
                // FileStream.Close might throw an exception, so put FileStream.Dispose in a separate try/finally
                fs.Dispose();
            }
        }
        catch (Exception ex)
        {
            // handle exceptions that were either thrown by opening the filestream, thrown by closing the filestream, or not caught by the inner try/catch
        }
    }

Taken to the extreme, though, this would be messy:

    void DoSomethingWithStream_PartDeux()
    {
        try
        {
            System.IO.FileStream fs = new System.IO.FileStream(@"C:\test.txt", System.IO.FileMode.Open);
            try
            {
                try
                {
                    // do something with the file stream

                }
                catch (Exception ex)
                {
                    // handle exceptions caused by reading the stream,
                    // if these need to be handled separately from exceptions caused by opening the stream
                }
                finally
                {
                    fs.Close();
                }
            }
            finally
            {
                // FileStream.Close might throw an exception, so put FileStream.Dispose in a separate try/finally
                fs.Dispose();
            }
        }
        catch (Exception ex)
        {
            // handle exceptions
        }
    }

Accessing a database could be even worse:

    void DoSomethingWithDatabase()
    {
        var connection = new System.Data.SqlClient.SqlConnection("Connect to mah database!");
        try
        {
            var command = new System.Data.SqlClient.SqlCommand("Get mah data!", connection);

            connection.Open();
            try
            {
                var reader = command.ExecuteReader();
                try
                {
                    try
                    {
                        // read data from data reader (duh)
                    }
                    finally
                    {
                        reader.Close();
                    }
                }
                finally
                {
                    reader.Dispose();
                }
            }
            finally
            {
                connection.Close();
            }
        }
        finally
        {
            connection.Dispose();
        }
    }

But then, in most cases I don't really see a need to explicitly close your streams / connections / data readers if you are going to dispose of them immediately afterward (unless you're just really paranoid). So, the database code above could just as easily be this:

    void DoSomethingWithDatabase_PartDeux()
    {
        using (var connection = new System.Data.SqlClient.SqlConnection("Connect to mah database!"))
        {
            var command = new System.Data.SqlClient.SqlCommand("Get mah data!", connection);

            connection.Open();
            using(var reader = command.ExecuteReader())
            {
                // read data from data reader (duh)
            }
        }
    }

Maybe I've just been tainted from coding with Dr. Wily's evil API. Using the initialize-variable-to-null trick doesn't work with his framework:

    void DoSomethingWithDrWilyEvilBoobyTrap()
    {
        Dr.Wily.Evil.BoobyTrap trap = null;
        try
        {
            trap = new Dr.Wily.Evil.BoobyTrap(Dr.Wily.Evil.Evilness.Very);

            // do something with booby trap
        }
        catch (Exception ex)
        {
            // handle exceptions
        }
        finally
        {
            if (trap != null) // Exception thrown here!
                trap.Dispose(); // Exception thrown here as well!
        }
    }

Here's a sneak peek at some of the souce code in his API:

public enum Evilness
{
    Slight,
    Moderate,
    Very,
}

class BoobyTrap : IDisposable
{
    public Evilness Evil { get; protected set; }

    public BoobyTrap(Evilness evil)
    {
        this.Evil = evil;
    }

    public void DoEvil()
    {
        // ... snip (sorry, it's just too evil) ...
    }

    public static bool IsNull(BoobyTrap instance)
    {
        throw new Exception("I bet you thought this function would work, didn't you?  Well it doesn't!  You should know whether or not your variables are null.  Quit asking me!");
    }

    public static bool operator !=(BoobyTrap x, object y)
    {
        if(y == null)
            throw new Exception("You cannot check if an instance of a BoobyTrap is null using the != operator.  Mwahahaha!!!");

        return x.Equals(y);
    }

    public static bool operator ==(BoobyTrap x, object y)
    {
        if (y == null)
            throw new Exception("You cannot check if an instance of a BoobyTrap is null using the == operator.  Mwahahaha!!!");

        return x.Equals(y);
    }

    #region IDisposable Members

    public void Dispose()
    {
        switch (this.Evil)
        {
            case Evilness.Moderate:
            case Evilness.Very:
                throw new Exception("This object is cursed.  You may not dispose of it.");
        }
    }

    #endregion
}
Dr. Wily's Apprentice