1

I'm trying to create a from to visualize and edit images, simple stuff like:

  • Drawing rectangles
  • Writing stuff
  • Cropping rectangles

But I'm encountering a lot of difficulties, for instance: If I try to load a file that doesn't exist or that can't be open (it's not a image), I'd like to show a MessageBox and then close the form. But the form doesn't close immediately (which may cause errors, since I may try to access properties of a file that wasn't open). Can you please tell me why? (check the Abort() Methods) The source code is in the end of the post.

I create my form inside with the following event:

private void button2_Click(object sender, EventArgs e)
        {
            Forms.AreaSelector areaSelector = new Forms.AreaSelector(LabelInput);
            areaSelector.ShowDialog();
        }

I'd like to show my form as a dialog so the user can't go back to the 'main window' without fishing the modifications to the image, that's why I'm using .ShowDialog(), instead of Show() I tried calling Close() and even Dispose() in my "Abort" method, but the form continues to load (by continues to load I mean that UpdateWindowSize() and UpdatePictureBox() are called regarless of what I do in Abort()).

And here's the actual form code.

Source:

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Windows.Forms;

namespace PeeAlyser.Forms
{
    public partial class AreaSelector : Form
    {
        #region Variables

        Bitmap originalImage, modifiedImage;
        string fileName;

        #endregion


        #region Constructors

        public AreaSelector(string fileName)
        {
            InitializeComponent();

            this.fileName = fileName;
        }

        #endregion

        private void AreaSelector_Load(object sender, EventArgs e)
        {
            TryToLoadImage();
            UpdateWindowSize();
            UpdatePictureBox();
        }

        #region Private Methods

        private void Abort()
        {
            this.DialogResult = System.Windows.Forms.DialogResult.Cancel;
            this.BeginInvoke(new MethodInvoker(this.Close));
            //this.Close();
            //this.Dispose();
            // GODAMNIT!
        }

        private void TryToLoadImage()
        {
            if (!System.IO.File.Exists(fileName))
            {
                MessageBox.Show("File not found.");
                Abort();
            }


            try { originalImage = (Bitmap)Bitmap.FromFile(fileName); }
            catch (Exception error)
            {
                MessageBox.Show("Error: " + Environment.NewLine +
                                error.ToString());
                Abort();
            }


            this.modifiedImage = new Bitmap(originalImage);
        }

        private void UpdateWindowSize()
        {
            int widthDifference = this.Width - pictureBox1.Width;
            int heightDifference = this.Height - pictureBox1.Height;

            Size windowSize = originalImage.Size;
            windowSize.Width += widthDifference;
            windowSize.Height += heightDifference;

            this.Size = this.MinimumSize = this.MaximumSize = windowSize;
            this.pictureBox1.Size = originalImage.Size;

            this.AdjustFormScrollbars(true);
        }

        private void UpdatePictureBox()
        {
            this.pictureBox1.Image = modifiedImage;
            this.Refresh();
        }

        #endregion
    }
}

Edit: I received a lot of suggestions to work around it. But Hans's answer not only corrected a design flaw in my correct (that also solved this issue) but also explained why this kind of issue happens (check his link). So I choose his answer. Feel free to close this quesiton, mods. And thanks for the help guys!

Trauer
  • 1,981
  • 2
  • 18
  • 40

3 Answers3

4

The Load event gets heavily over-used in Winforms. A hangup that it inherited from VB6, its predecessor, where it was a very important because that's where you put any initialization code. Which made it the default event, too easy to use.

But not in .NET, initialization is done in the constructor of a class. The Load event is too late, the train left the station and has gathered considerable speed already. Trying to stop it is difficult, throwing an exception has no effect for example. And in fact is very dangerous. The Load event should only ever be used when you need to know the size and location of the window. That's pretty rare, although it looks like you have a use.

You need to stop that train before it gets going, your TryToLoadImage() call belongs in the constructor. Now it is dead-simple, you do the normal thing you do in C# when something that normally works cannot work, you throw an exception. And catch it at the ShowDialog() call site. Easy peasy.

Community
  • 1
  • 1
Hans Passant
  • 922,412
  • 146
  • 1,693
  • 2,536
  • Thanks for the metaphor and the suggestion. I'll move my TryToLoadImage() to the constructor and will reply with the results shortly. But why, exactly, "the train can't be stopped"? By the way... It does make sense to ._. allocate stuff in the constructor... My bad, hehe – Trauer Jan 12 '15 at 22:51
  • The train is the native window that the operating system creates. It was created and initialized, just not visible yet. The Load event is the "I'm about to get visible" notification. You can call Close() or set the DialogResult property, but that doesn't prevent the window from flashing on the screen briefly. And raise some other events that might bomb because your bitmap didn't get loaded. Windows doesn't have an option to cancel the notification, most visible by the Load event not having a CancelEventArgs argument. – Hans Passant Jan 12 '15 at 22:55
0

You can make TryToLoadImage boolean (substitute Adort by return false) and run it like this:

if(TryToLoadImage())
{
        UpdateWindowSize();
        UpdatePictureBox();
}
else Close();
Nadia Chibrikova
  • 4,916
  • 1
  • 15
  • 17
0

The best option would be to avoid the "doubtful part" in the load event.

However, if you want to carry out the operations in the Load Event itself, I'd suggest a simple procedure.

Use an integer variable called access and set it by default to 1

As you mentioned, you'd like to display a messagebox and then close it. Just before the messagebox line, set the value of the access variable to 0.

Enclose the post-messagebox code into an If-Else block.

The code that you'll need is:-

    using System;
    using System.Collections.Generic;
    using System.ComponentModel;
    using System.Data;
    using System.Drawing;
    using System.Linq;
    using System.Text;
    using System.Threading.Tasks;
    using System.Windows.Forms;

    namespace PeeAlyser.Forms
    {
      public partial class AreaSelector : Form
       {
    #region Variables

    Bitmap originalImage, modifiedImage;
    string fileName;

    #endregion


    #region Constructors

    public AreaSelector(string fileName)
    {
        InitializeComponent();

        this.fileName = fileName;
    }

    #endregion

public int access=1;

    private void AreaSelector_Load(object sender, EventArgs e)
    {
        TryToLoadImage();
        if(access==1)
    {
        UpdateWindowSize();
            UpdatePictureBox();
    }
    }

    #region Private Methods

    private void Abort()
    {
        this.DialogResult = System.Windows.Forms.DialogResult.Cancel;
        this.BeginInvoke(new MethodInvoker(this.Close));
        //this.Close();
        //this.Dispose();
        // GODAMNIT!
    }

    private void TryToLoadImage()
    {
        if (!System.IO.File.Exists(fileName))
        {
    access=0;
            MessageBox.Show("File not found.");
            Abort();
        }


        try { originalImage = (Bitmap)Bitmap.FromFile(fileName); }
        catch (Exception error)
        {
            MessageBox.Show("Error: " + Environment.NewLine +
                            error.ToString());
            Abort();
        }


        this.modifiedImage = new Bitmap(originalImage);
    }

    private void UpdateWindowSize()
    {
        int widthDifference = this.Width - pictureBox1.Width;
        int heightDifference = this.Height - pictureBox1.Height;

        Size windowSize = originalImage.Size;
        windowSize.Width += widthDifference;
        windowSize.Height += heightDifference;

        this.Size = this.MinimumSize = this.MaximumSize = windowSize;
        this.pictureBox1.Size = originalImage.Size;

        this.AdjustFormScrollbars(true);
    }

    private void UpdatePictureBox()
    {
        this.pictureBox1.Image = modifiedImage;
        this.Refresh();
    }

    #endregion
}
}
Anmol Kumar
  • 90
  • 1
  • 12