Lesson 18 Exercise 3

SamW · November 23rd, 2012, 05:19 AM

Hi,

I was comparing my code to that in your solution, and instead of declaring the 'shouldContinue' variable, I used the following:

Code:

        private void Form1_FormClosing(object sender, FormClosingEventArgs e)
        {
            // Decides whether we should continue with this operation.
            e.Cancel = true;

            // See if we have unsaved changes.
            if (contentRichTextBox.Modified)
            {
                // Ask the user if we should save the changes.
                DialogResult result =
                    MessageBox.Show(
                        "There are unsaved changes.\n" +
                        "Do you want to save the changes?",
                        "Save Changes?",
                        MessageBoxButtons.YesNoCancel,
                        MessageBoxIcon.Question);

                // See if we should save the changes.
                if (result == DialogResult.Yes)
                {
                    // Save the changes here.
                    contentRichTextBox.SaveFile("Test.rtf", RichTextBoxStreamType.RichText);

                    // It's now safe to continue.
                    e.Cancel = false;
                }
                else if (result == DialogResult.No)
                {
                    // Just continue and that will discard the changes.
                    e.Cancel = false;
                }
                else
                {
                    // The user wants to cancel.
                    // Don't continue.
                    e.Cancel = true;
                }
            }
            else
            {
                // No unsaved changes so it's safe to continue.
                e.Cancel = false;
            }

        }

Is there anything wrong with using e.Cancel directly for the if/else tests rather than the shouldContinue variable, or is it simply a case of your code being more explicit as to what it's doing? I'm trying to learn best practice here, so I'd appreciate your advice.

Rod Stephens · November 23rd, 2012, 12:23 PM

Just out of habit I usually only set e.Cancel once and only if I need to. If I want it to take the default value, I leave it alone.

But there's absolutely nothing wrong with the way you're doing it. In fact, now that I think about it, I'm not sure why I have this habit.

SamW · November 28th, 2012, 07:51 AM

Thanks Rod! I know everyone has their own habits, that's the challenge for me learning - I read through other people's code and there are numerous ways of doing the same thing so I get confused. I should probably drop the habit of wanting to be Right, in favour of just making sure my code is clear and has comments as needed.

EDIT:

Aha! Now I see the value of your way! When you come to do Exercise 20.4, pasting my version into an IsDataSafe method throws an error because e hasn't been declared.

Rod Stephens · November 28th, 2012, 12:15 PM

You're right that for any given problem there are many solutions. You can try to find the "best" solution but at some point you need to decide when a solution is good enough and stop there. Otherwise you can tweak the code endlessly and never finish. And does it really matter if a program takes 10 milliseconds or 9 to accomplish a task?

I usually favor readability over performance. If the code is 10% slower than the perfect solution but the faster solution is to complicated to debug and maintain, then the slower solution is better.

Often that also means going with what you're used to. Over time you develop habits and if you stick with them you're more likely to not make mistakes.

MinusZero · December 6th, 2012, 07:24 PM

I have modified the code for the yes result of the dialog box so that instead of saving to a hard coded test.rtf file, it shows the save as dialog.

I used the code that i used for the Save As menu option.

I have a problem though. The dialog shows ok if I want to save the modifications, I click yes and it shows the save as dialog box, however, if after showing the dialog box i click cancel, it crashes. It has an error ArgumentException was unhandled - Empty pathname is not legal. Below is the code i am using. The red text is the line where the error crashes to.

Code:

private void SimpleEditForm_FormClosing(object sender, FormClosingEventArgs e)
        {
            //decides whether to continue with the operation
            bool shouldContinue;

            // Check for unsaved changes.
            if (contentRichTextBox.Modified)
            {
                // Ask the user if we should save the changes.
                DialogResult result =
                    MessageBox.Show(
                        "There are unsaved changes.\n" +
                        "Do you want to save the changes?",
                        "Save Changes?",
                        MessageBoxButtons.YesNoCancel,
                        MessageBoxIcon.Question);

                // See if we should save the changes.
                if (result == DialogResult.Yes)
                {
                    // Save the changes.
                    saveFileDialog1.ShowDialog();
                    contentRichTextBox.SaveFile(saveFileDialog1.FileName, RichTextBoxStreamType.RichText);
                    openFileDialog1.FilterIndex = saveFileDialog1.FilterIndex;

                    // It's now safe to continue.
                    shouldContinue = true;
                }
                else if (result == DialogResult.No)
                {
                    // continue and discard the changes.
                    shouldContinue = true;
                }
                else
                {
                    // The user wants to cancel.
                    // Don't continue.
                    shouldContinue = false;
                }
            }
            else
            {
                // No unsaved changes so it's safe to continue.
                shouldContinue = true;
            }

            // See if it's safe to continue.
            e.Cancel = (!shouldContinue);
        }

I figure that it means that it wants a filename, but why when i cancel the save as dialog isnt it just cancelling? Is it because its still part of the save modifications dialog box? Do i need a new 'if' statement to cover the save as dialog?

Rod Stephens · December 7th, 2012, 04:04 PM

I think this is the key piece of code:

Code:

if (result == DialogResult.Yes)
{
    // Save the changes.
    saveFileDialog1.ShowDialog();
    contentRichTextBox.SaveFile(saveFileDialog1.FileName,
        RichTextBoxStreamType.RichText);
    openFileDialog1.FilterIndex = saveFileDialog1.FilterIndex;

    // It's now safe to continue.
    shouldContinue = true;
}

You displayed the "Save Changes?" dialog (not shown in this code) and the user clicked Yes. Then the red code above displays the Save File dialog and the user clicks Cancel. The red code needs to check the Save File dialog's return result to see if the user clicked Cancel.

I think it might look something like this:

Code:

if (result == DialogResult.Yes)
{
    // Save the changes.
    if (saveFileDialog1.ShowDialog() == DialogResult.Cancel)
    {
        // Cancel everything.
        shouldContinue = false;
    }
    else
    {
        contentRichTextBox.SaveFile(saveFileDialog1.FileName,
            RichTextBoxStreamType.RichText);
        openFileDialog1.FilterIndex = saveFileDialog1.FilterIndex;

        // It's now safe to continue.
        shouldContinue = true;
    }
}

I'm typing without trying this in Visual Studio so I may have the details wrong but I think this is the sort of thing you'll need to do.

MinusZero · December 11th, 2012, 12:55 AM

I will try it out and let you know