Streamreader file.Readline properly closing the stream

Question

Is this OK? (.Net 4.5)

try
{
 StreamReader sr = = new StreamReader(ValidFilePathName);
 line = file.ReadLine();
 While (line != null)
 {
 line = file.ReadLine();
 }
 sr.Close();
}
catch
{
 // blah Do nothing
}

I feel like there is something wrong with my pattern here. My concern is that when there is a file exception, does the stream get closed properly? I was taught to use using to set the stream, but Code Analysis tool does not like it. Perhaps there is something else. It seems to read the file OK, but I remember needing to use File stream and stream reader in the past... Was there a good reason for combining the two?

• Vsevolod Goloviznin: You need to close your stream in the finally block. Why does your code analysis tool not like the using statement?
• amalgamate: I think it is when I use nested using statements where this code is not an example of that. So In the finally block... yes but that means I need to... OK I think I have it. Thanks.
• amalgamate: So that means I also need to move the initialization of the SR outside the try block.
• Vsevolod Goloviznin: Yes. What tool do you use? Multiple using statements should be ok
• Vsevolod Goloviznin: I guess you can ask a new question with the code that produces it to get some clarification on reasons behind usch behavior of the tool

Answer 1

You always need to dispose of objects in the finally close to ensure that they are disposed correctly.

StreamReader sr = null;
try
{
 sr = = new StreamReader(ValidFilePathName);
 line = file.ReadLine();
 While (line != null)
 {
 line = file.ReadLine();
 }
}
catch
{
 // blah Do nothing
}
finally
{
 if (sr != null)
 {
 sr.Close();
 } 
}

• amalgamate: Error: Use of unassigned local variable SR in finally block. I can assign null at the first line?
• Vsevolod Goloviznin: Yeah, I've updated the code. I've also included the check in the finally just in case StreamReader will not be initialized.

Answer 2

I know there is already an accepted answer here, but have you ever utilized the using statement? This statement will automatically handle all of your disposal for you. And yes, it will take care of it on exceptions. Check out the link HERE and my example below.

try
{
 using (StreamReader sr = new StreamReader(ValidFilePathName))
 {
 line = file.ReadLine();
 While (line != null)
 {
 line = file.ReadLine();
 }
 }
}
catch (Exception ex)
{
 string myException = ex.ToString();
}

• amalgamate: That was where the re-factoring started... actually. CA2202 is a code analysis warning (starting 2012?). That warning does not like this way of disposing of idisposable objects. One can choose to ignore this warning. I am undecided my self, but I (secretly) wanted to be able to properly handle file access without the warning. I think this answer is highly useful as part of the conversation, and so +1 for that.
• Volearix: Interesting. I don't receive that warning when using it, but I will have to keep an eye out for sure. Never had any actual errors due to it, I know that much! Thanks for the +1 though!
• amalgamate: You have to run the vs code analysis tool to get the warning. I think it is in vs2012, but it is definitely in vs2013. Of course I don't think it causes any errors. I think Microsoft is afraid of idisaposable objects being written or overridden poorly.