Question

The c# code below is by no means ideal and I'm really just looking for advice and suggestions as to how I could best refactor this and make the code safer.

Basically there is a class variable that stores the security check stage value (initialised to 0). When a button in my application is pressed the code below is run to check that the user has permission to access their account screen. Depending upon a method parameter the appropriate handler method is called, which shows a PIN entry user control to the user (this user control is a custom control that appears full screen and top-most). While the handler code is running the code shown below is calling Application.DoEvents in a do while loop to keep everything responsive while the user is entering their PIN. If the do while loop wasn't there the screen that the user is trying to access would appear on top of the PIN entry screen before we have a chance to valid that the user PIN is correct. When the PIN entry has passed the security check stage variable is set to 1, allowing the account screen to be shown.

            try
            {
                this.Cursor = Cursors.WaitCursor;

                Application.DoEvents();

                SecurityCheckStage = 0;

                Security             = new tskSecurity(true);
                Security.TaskUpdate += new TaskUpdateHandler(_handler);

                TaskManager.AddTask(Security, true);

                this.Cursor = Cursors.Default;

                // Wait until security check has passed before showing account screen
                do
                {
                    Application.DoEvents();
                    System.Threading.Thread.Sleep(100);
                }
                while (SecurityCheckStage == 0);

                if (SecurityCheckStage == 1) ShowAccountScreen();

                return false;
            }
            catch
            {
                throw;
            }
            finally
            {
                this.Cursor = Cursors.Default;
            }

I am aware that calling Application.DoEvents() in a loop is not good practice, so I really want to rework this code to make it better.

Any help would be much appreciated. Please bear in mind that the solution to the problem must work with the .NET 3.5 Framework.

Was it helpful?

Solution

Use a System.Windows.Forms.Timer... for example:

//...

    timer.Tick += TimerEventProcessor;
    timer.Start();
//..
private void TimerEventProcessor(Object sender, EventArgs myEventArgs)
{
    if (SecurityCheckStage == 1)
    {
        var timer = (Timer) sender;
        timer.Stop();
        ShowAccountScreen();
    }
}
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top