Results 1 to 6 of 6

Thread: Refctoring if statements, a better way?

  1. #1

    Thread Starter
    Fanatic Member
    Join Date
    Aug 2005
    Location
    Wisconsin
    Posts
    788

    Refctoring if statements, a better way?

    I want to check if a group of backgroundworkers are processing when a user may click on the button that starts them. Is there a better way than this?
    Code:
                if (bgwCheckFinished.IsBusy)
                {
                    MessageBox.Show("There is another process running, please wait until it finishes.");
                    return;
                }
                if (bgwBackup.IsBusy)
                {
                    MessageBox.Show("There is another process running, please wait until it finishes.");
                    return;
                }
                if (bgwMain.IsBusy)
                {
                    MessageBox.Show("There is another process running, please wait until it finishes.");
                    return;
                }
                if (bgwRep.IsBusy)
                {
                    MessageBox.Show("There is another process running, please wait until it finishes.");
                    return;
                }

  2. #2
    PowerPoster kfcSmitty's Avatar
    Join Date
    May 2005
    Posts
    2,248

    Re: Refctoring if statements, a better way?

    Since all of the objects are the same, why not use a list with one if statement?

    Code:
    List<BackgroundWorker> workers = new List<BackgroundWorker>()
    {
      bgwCheckFinished,
      bgwBackup,
      bgwMain,
      bgwRep
    };
    
    foreach(BackgroundWorker worker in workers)
    {
      if (worker.IsBusy)
      {
         MessageBox.Show("There is another process running, please wait until it finishes.");
         return;
      }
    }
    Then you simply add the new worker to the list.

    Note that I wrote this in the quickreply window, so it might not be syntatically correct.

  3. #3
    Frenzied Member
    Join Date
    Jan 2009
    Location
    Watch Window(Shift+f9)
    Posts
    1,879

    Re: Refctoring if statements, a better way?

    I will suggest to user Dictionary rather than using List.try the following way .
    Code:
    Dictionary<BackgroundWorker> workers = new Dictionary<BackgroundWorker>(){
      bgwCheckFinished,
      bgwBackup,
      bgwMain,
      bgwRep
    };
    
    foreach(BackgroundWorker worker in workers){
      if (worker.IsBusy)  {
         MessageBox.Show("There is another process running, please wait until it finishes.");
         return;
    
       else {
         'Do whatever you want
      }
    }

  4. #4

    Thread Starter
    Fanatic Member
    Join Date
    Aug 2005
    Location
    Wisconsin
    Posts
    788

    Re: Refctoring if statements, a better way?

    I would like to show only one message and then get out. If any of them are busy I cannot let it continue.firoz.raj suggestion looks like it would do the trick. Which way is more efficient?

  5. #5
    PowerPoster kfcSmitty's Avatar
    Join Date
    May 2005
    Posts
    2,248

    Re: Refctoring if statements, a better way?

    I'm pretty sure firoz.raj's way won't work since a dictionary requires a key and a value. While a dictionary does have better performance than a list at around 4 or 5 items and on (containsKey vs looping), I believe the minimal slowdown of using a list is worth having more readable and maintainable code.

    What you could do is:

    Code:
    List<BackgroundWorker> workers = new List<BackgroundWorker>()
    {
      bgwCheckFinished,
      bgwBackup,
      bgwMain,
      bgwRep
    };
    
    BackgroundWorker busyWorker = workers.Where(b => b.IsBusy).FirstOrDefault();
    
    if (busyWorker != null)
    {
      MessageBox.Show("There is another process running, please wait until it finishes.");
    }
    This would require using System.Linq;
    Last edited by kfcSmitty; Nov 1st, 2013 at 12:19 PM. Reason: added clarification on a statement

  6. #6
    Frenzied Member
    Join Date
    Jan 2009
    Location
    Watch Window(Shift+f9)
    Posts
    1,879

    Re: Refctoring if statements, a better way?

    I would like to show only one message and then get out. If any of them are busy I cannot let it continue.firoz.raj suggestion looks like it would do the trick. Which way is more efficient?
    the alternate way I would suggest .the following .
    Code:
    
    List<BackgroundWorker> workers = new List<BackgroundWorker>()
    {
      bgwCheckFinished,
      bgwBackup,
      bgwMain,
      bgwRep
    };
    
    BackgroundWorker busyWorker = workers.Where(b => b.IsBusy).FirstOrDefault();
    
    if (workders.isbusy(){
      MessageBox.Show("There is another process running, please wait until it finishes.");
      return
    elseif {
      'Do what you want
    
     }
    }

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •  



Click Here to Expand Forum to Full Width