Am I using try catch finally right?

[DeanMc]

Hi Guys,

I am trying to cover all my bases here with an application I am creating. As it will be released to the general community I have to account for user error so I am implementing structured error handling. What I want to know is:

A: am I using this correctly
B: if I but the finally block at the end of a method does it skip over any code after the catch block and close the method?

        private void OpenFileForReading(string Path)
        {
            try
            {
                stream = new FileStream(Path, FileMode.Open);
                reader = new BinaryReader(stream);
            }

            catch (Exception e)
            {
                ListOfErrors.Add(e.ToString());
            }

            finally
            {
                reader.Close();
                reader = null;

                stream.Close();
                stream = null;
            }
        }

[techgnome]

in the finally, you should first check 1) that the object exists.. and 2) that it is open.... then close it. if the error happens because the stream couldn't be opened... then the object is likely to be null... which means 1) you can't call close on your stream and 2) your reader will have never been created, so it too will be null.

In regards to B ... Try catch finally have to appear together... you can't have extraneous code between the Catch and Finally (or at least I don't think you can, nor would that make sense.)

-tg

[DeanMc]

Ah yes that would make sense for A. as for B so everything should be in the Try Part?

[techgnome]

That's up to you... generally most people like to try to keep their try blocks as small as possible... but it depends on how granular you want to trap for errors. Obviously try every line one by one would not only be tedious, but over kill. By the same token you don't want a 100-line Try blck where a dozen different things could go wrong either. since you are closing your streams in the finally, obviously, anything that comes after that isn't going to have the streams open... There's no clear answer. Actually there is: It depends.

-tg

[jmcilhinney]

'finally' blocks are redundant in many cases these days. If all you're doing in a 'finally' block is disposing objects then you should probably be using a 'using' block, no pun intended.

        private void OpenFileForReading(string Path)
        {
            try
            {
                using (stream = new FileStream(Path, FileMode.Open))
                {
                    using (reader = new BinaryReader(stream))
                    {
                        // ...
                    }
                }
            }
            catch (Exception e)
            {
                ListOfErrors.Add(e.ToString());
            }
        }

No need to null your variables because they are scoped by the 'using' block and no need to test for null before disposing because disposal is implicit.

Note that 'using' is only for objects that implement IDisposable, so you would still need to use a 'finally' block where there was work other than disposing objects to do.

[DeanMc]

Cheers Guys!