Is this code wrong?

This is a code snippet from lesson 2 in the book ... when I typed it in I thought it would fail, if there was a problem finding the file.

The problem I saw is the StreamReader will try and open the file before the Try statement, so if there is an error it would not be caught.

The book said to put the StreamReader outside the Try Statement, so the finally block can close the file, since it could not see 'tr' if it was declared inside the Try statement.

private void showButton_Click(object sender, EventArgs e)
        {
            TextReader  tr = new StreamReader(locationTextBox.Text);
            try
            {
                displayTextBox.Text = tr.ReadToEnd();
            }
            catch (Exception ex)
            {
                MessageBox.Show(ex.Message);
            }
            finally
            {
                 tr.Close();
            }
        }

I worked around it by doing this, but would like to know if I missed something in the first go around or if there would be another way to do it.

private void showButton_Click(object sender, EventArgs e)
        {
            TextReader tr=null;
            try
            {
                tr = new StreamReader(locationTextBox.Text);
                displayTextBox.Text = tr.ReadToEnd();
            }
            catch (Exception ex)
            {
                MessageBox.Show(ex.Message);
            }
            finally
            {
                try
                {
                    tr.Close();
                }
                catch { }
            }
        }

Find more posts tagged with

Comments

dynamik

Wouldn't this always work as long as locationTextBox.Text existed?

TextReader  tr = new StreamReader(locationTextBox.Text);

It seems like that just creates a new StreamReader from that value, and it would work whether or not you referenced a valid file.

Wouldn't a missing file fail at this line when it actually tries to read it?

displayTextBox.Text = tr.ReadToEnd();

That seems like the point of putting it in try, but I might be completely wrong. I'm just a hobbyist

MidLiveUpgrade

dynamik wrote:
Wouldn't a missing file fail at this line when it actually tries to read it?
displayTextBox.Text = tr.ReadToEnd();

That is what it appears the book was assuming, but the missing file error was being generated when the stream was being created.

Dave.

dynamik

Interesting. Nice work-around though

Also, do you have the errata for the book you're using? That'll usually save you a ton of headaches as you progress through it.

MidLiveUpgrade

dynamik wrote:

Also, do you have the errata for the book you're using? That'll usually save you a ton of headaches as you progress through it.

No I don't, but it is something I should try and locate.

MCPWannabe

Hello There. Very good question.

// C#

try
{
TextReader tr = new StreamReader(locationTextBox.Text);
try
{ displayTextBox.Text = tr.ReadToEnd(); }
catch (Exception ex)
{ MessageBox.Show(ex.Message); }
finally
{ tr.Close(); }
}
catch (Exception ex)
{ MessageBox.Show(ex.Message); }

You are correct in that your first example would fail to catch an error upon initialization, but that is because you left a critical part out of your copied statement. To get around this problem, you need to Nest a Try statement within another Try statement. Up above, the variable retains scope because, the Try, Catch, and Finally methods are nested underneath the first 'try' method. There is a second catch statement, but this is catching an exception. Notice that there is no value for closing the variable when the scope of the second try/catch/finally block of sequences end. This will still give your variable scope. See example above from book..

Your second example worked due to the fact that you declared the variable, then initialized it within a Try/Catch block.

Overall, good job.. This was just a mistake from not copying the full example from the book.

[/quote]

MidLiveUpgrade

This was just a mistake from not copying the full example from the book.

Ok, I see the problem ... your are right, the book is correct.

In this case I followed the instructions, which said to use the code supplied on the CD, which was wrong.

In the future I need to cross reference the code supplied with the book.

Thanks,
Dave.

MCPWannabe

I was rather impressed with your work around. Regarding the solution errors, it is very common in Microsoft Programming books. For the e-learning, Microsoft pays a technical reviewer for each course, $150 an hour (currently), to spend 20 hours going over a 2 hour course.

I'm always seeing advertisements for programming technical reviewers. I haven't seen any advertisements for the MS Pressbooks yet.

Next year, I plan on becoming a technical reviewer also, but I have enough on my plate now. Hopefully, they will start catching some of those errors.

For now, you may have to just do what you did and what I did to learn the material. When you see something that looks funny, post it on a forum and ask others for help. More than likely, you'll be noticing something that isn't correct. Consequently, the few wrong solutions really increased my understanding of the material. Something about having to work for the answer really helps!

mvastarelli

MCPWannabe wrote:

For now, you may have to just do what you did and what I did to learn the material. When you see something that looks funny, post it on a forum and ask others for help. More than likely, you'll be noticing something that isn't correct. Consequently, the few wrong solutions really increased my understanding of the material. Something about having to work for the answer really helps!

If I may nitpick a little. While the workaround proposed by the OP does indeed work, if that code had difficulty finding the file it would ultimately throw two exceptions. One when creating the stream reader and the second when attempting to close the non-existent stream reader.

Exceptions are expensive constructs and it's generally good practice to craft the code to avoid using them whenever possible. I would remove the nested try block when closing the stream and replace it with an

if(tr!=null) { tr.Close(); }

If the stream can't be created it will retain it's original null value, and if it did create successfully (i.e. it's not null) we can safely assume that we can close it.