2

So I am fairly new to C# (not programming in general) and I wrote some code with nearly Zero error handling to make a proof of concept for running in VS only. Now I am getting ready to build some error handling into the application so that the application can be deployed. So now I am having issues figuring out how/where to declare variables so that I can properly configure a Try Catch block.

So with primitive types I just create the variable outside of the block

Before:

String TheString = "Some times I break";
SomeFunctionThatBreaks(TheString);

SomeFunctionThatDoesntBreak(TheString);

After:

String TheString ="";
Try
{
TheString="Some Times I break";
SomeFunctionThatBreaks(TheString);
}
Catch
{
MessageBox.Show("Error")
return false;
}
SomeFunctionThatDoesntBreak(TheString);

However with complex types such as FileStream I am not sure of the proper way to create a EMPTY variable for later use:

Before:

        FileStream SourceFile = File.OpenRead(TheFile);
        StreamReader sr ; = new StreamReader(SourceFile);            
            char[] block = new char[3];
            byte[] header = new byte[6];
            SourceFile.Read(header, 0, 6);
            SourceFile.Seek(0, 0);
            encoding = (header[1] == 0 && header[3] == 0 && header[5] == 0) ? Encoding.Unicode : Encoding.UTF8;
            sr.ReadBlock(block, 0, 3);
            String sBlock = new String(block);
            SourceFile.Seek(0, 0);
    if(sBlock=="ABC")
    {
MyFunction(SourceFile);
    }

Causes compile errors:

    FileStream SourceFile ;
    String sBlock ="";
    Encoding encoding;
    StreamReader sr;

    try
    {
        SourceFile = File.OpenRead(TheFile);
        sr = new StreamReader(SourceFile);
        char[] block = new char[3];
        byte[] header = new byte[6];
        SourceFile.Read(header, 0, 6);
        SourceFile.Seek(0, 0);
        encoding = (header[1] == 0 && header[3] == 0 && header[5] == 0) ? Encoding.Unicode : Encoding.UTF8;
        sr.ReadBlock(block, 0, 3);
        sBlock = new String(block);
        SourceFile.Seek(0, 0);

    }
    catch (Exception ex)
    {
        String Error = "Error Accessing: " + TheFile  + " System Message: " + ex.Message;
        EventLog.LogEvent(dtmLogging.LogEventType.Error, Error);
        MessageError(Error, "MyFunction()");
    }
    if(sBlock=="ABC")
    {
MyFunction(SourceFile);  //THIS LINE DOES NOT COMPILE: Use of unassigned variables
    }

Proposed Change: //If I make this change application appears to work fine, but I am not sure if this is "Proper"

    FileStream SourceFile =null;
    String sBlock ="";
    Encoding encoding = null;
    StreamReader sr = null;

Thank you for any help

Cade
  • 97
  • 2
  • 8
  • This is a red error that will not allow me to compile or debug. I am aware of the difference – Cade Jan 21 '15 at 18:10
  • I don't understand why you don't do what you did before - assign the variable(s) outside the try block. Unless for complex types you think there may be an exception thrown during construction or initialization. In that case the unassigned var error is pointing out a valid problem - you don't know if the variable was constructed/initialized after the try block. – hatchet - done with SOverflow Jan 21 '15 at 18:15
  • I have edited code to include line that causes error – Cade Jan 21 '15 at 18:17
  • in answer to if this is 'proper' - there are other similar questions which have this as the proposed answer - e.g. http://stackoverflow.com/questions/6213113/fixing-the-use-of-unassigned-local-variable-with-a-null-assignment-why – NDJ Jan 21 '15 at 18:24
  • Do you have a _specific_ problem? Initializing variables to `null` will work, and is mandatory if you need to use them again outside the `try` part of the `try`/`catch`. If you only need them inside the `try` part, then they should be declared and initialized there. Note that this has nothing to do with "primitive" vs. other types; every variable must be initialized to _something_. Note that if you initialize to `null`, you then have to check for `null` before you attempt to use the variable in a way that requires a non-`null` value! – Peter Duniho Jan 21 '15 at 18:35
  • Thank you NDJ and Peter. I am new to C# and wanted to make sure this is proper before I adjust all my code to work this way. Thank you – Cade Jan 21 '15 at 18:41

2 Answers2

3

I would recommend not setting the variable to null in the declaration, at least in a case such as your sample code. In this case, setting it to null actually obscures another problem with the code as written: the catch block allows the method to keep executing (and attempting to use the FileStream object) even though something went wrong and the FileStream object is likely in an unknown state and shouldn't be used.

The reason you were seeing the "red error" initially was because the code could "fall through" the catch block. If you had added a return or throw at the end of the catch block to return control to the caller, the red error would have disappeared and you wouldn't have needed to set the variable to null.

To answer your original question, declaring the variables as you did can be a reasonable way to structure the code. A better way is to incorporate all your actions on the FileStream within the try block so that the variables can be defined and referenced all within that same block. If the try block starts to get too big, it's probably an indication that you should refactor some of its contents out into smaller utility methods that can be called from within the try block.

And one related point: you should have using declarations around objects that need to be closed or otherwise cleaned up after use (i.e. anything that implements the IDisposable interface). This ensures that files get closed and other resources released regardless what happens in the rest of the code.

For example:

try
{
    using (FileStream SourceFile = File.OpenRead(TheFile))
    using (StreamReader sr = new StreamReader(SourceFile))
    {
        char[] block = new char[3];
        byte[] header = new byte[6];
        SourceFile.Read(header, 0, 6);
        SourceFile.Seek(0, 0);
        Encoding encoding = (header[1] == 0 && header[3] == 0 && header[5] == 0) ? Encoding.Unicode : Encoding.UTF8;
        sr.ReadBlock(block, 0, 3);
        String sBlock = new String(block);
        SourceFile.Seek(0, 0);

        if (sBlock=="ABC")
        {
            MyFunction(SourceFile);
        }

        // And do anything else you want with SourceFile & sr here
    }
}
catch (Exception ex)
{
    String Error = "Error Accessing: " + TheFile  + " System Message: " + ex.Message;
    EventLog.LogEvent(dtmLogging.LogEventType.Error, Error);
    MessageError(Error, "MyFunction()");
}

Hope this helps.

The Mad Coder
  • 161
  • 1
  • 3
0

SourceFile has been defined in the Try block, it won't be in scope later. It acts as a local variable meant for just Try block.

The below link too will help.

Why aren't variables declared in "try" in scope in "catch" or "finally"?

Community
  • 1
  • 1
Anurag
  • 552
  • 9
  • 31