[immeraufdemhund]

In an effort to refactor some of my code I decided that I should look into 2 different things. One making an abstract super panel for my self since I have 12 panels that all look roughly the same but perform very different unrelated tasks. The other thing I wanted to do was to start using multithreading. I read up on a tutorial here, and tried my best to port it over to a much smaller test project to see how well this would work. But when I kick off the background worker and it goes to change a label in this panel it gives me the Cross-Thread operation not valid: Control '' accessed from a thread other than the thread it was created on. I'm glad I decided to try this out on a much smaller project than my main project that I want to incorporate this on. So I'll post up my 3 classes. I hope someone can help me on this.

MainForm - 2 buttons and 1 InformationPanel. InformationPanel extends AbstractSuperPanel which extends Panel.

    public partial class MainForm : Form
    {
        public MainForm()
        {
            InitializeComponent();
        }
        
        void Button1Click(object sender, EventArgs e)
        {
            backgroundWorker1.RunWorkerAsync();
        }
        
        void Button2Click(object sender, EventArgs e)
        {
        }
        
        void BackgroundWorker1DoWork(object sender, System.ComponentModel.DoWorkEventArgs e)
        {
            informationPanel1.StartWorking();
        }
    }

AbstractPanel. This just sets up the class as it should look in the main form. When extended the child class just implements it's own controls.

        public Label PanelLabel = new Label();
        public AbstractPanel()
        {
            PanelLabel.Location = new Point(0,0);
            PanelLabel.Font = new Font("Arial", 10F, FontStyle.Bold, GraphicsUnit.Point, 0x00);
            this.SuspendLayout();
            this.BorderStyle = BorderStyle.FixedSingle;
            this.BackColor = SystemColors.Control;
            this.Font = new Font("Arial", 8.25F, FontStyle.Regular, GraphicsUnit.Point, ((byte)(0)));
            this.Size = new Size(200, 200);
            this.Controls.Add(PanelLabel);
            InitializeComponent();
            this.ResumeLayout(false);
        }
        public abstract void InitializeComponent();
        public abstract void StartWorking();
    }

And now my InformationPanel.

    public class InformationPanel : AbstractPanel
    {
        private Label WorkLabel = new Label();
        public InformationPanel()
        {
            
        }
        public override void StartWorking()
        {
            AsyncOperation operation = AsyncOperationManager.CreateOperation(null);
            ThreadPool.QueueUserWorkItem(new WaitCallback(StartLabels));
        }
        private void StartLabels(object stateInfo)
        {
            Thread.Sleep(1000);
            WorkLabel.Text = "Finished";
        }
        public override void InitializeComponent()
        {
            PanelLabel.Text = "Information";
            WorkLabel.Font = base.Font;
            WorkLabel.Location = new Point(2,22);
            this.Controls.Add(WorkLabel);
        }
    }

As you can see the concept is easy. I just want to populate the child panels labels with the StartWorking() method. In my main project I have 12 similar panels that will eventually extend AbstractPanel as it does here so that I can loop through my controls in my main form from the background worker and tell it to work. I have it working somewhat in my main program, but I'm doing it a little differently. I also haven't implemented Abstract panel to all my panels yet. Maybe I was doing it right in this version, I just don't know. All I know is it that it works. Keep in mind that none of the panels talk to each other and can be thought of almost as 12 individual programs. Here is how I currently do it.

        private void Form1_Load(object sender, EventArgs e)
        {
            backgroundWorker1.RunWorkerAsync();
            imagerDataPanel1.OpenImager();
        }
        
        void BackgroundWorker1DoWork(object sender, System.ComponentModel.DoWorkEventArgs e)
        {
                unitInformationPanel1.PopulateUnitInformation();
                batteryInformationPanel1.InitializeBatteries();
                magStripeReaderPanel1.SetupPointOfSale();
        }

Mind you those 3 panels that have different methods will change when they extend the Abstract panel to be StartWork(). unitInformationPanel is the main culprit for time consuming operations. It takes it about 3-5 seconds to get everything it needs.

Thank you for your time.

[tlhIn`toq]

Quote

But when I kick off the background worker and it goes to change a label in this panel

I'll stop you right there. That underlying principal for your program is the problem and just won't work. The worker is a different thread than the one that created the GUI.

Background threads do not, ever, directly access anything that they didn't make. Period. Accept that and start re-designing.

Background worker raises events. You main class that makes this background worker has to subscribe to those events. Then that thread can update its GUI.

This is how is it meant to work. Each 'thing' in a program really should have only one purpose in life and cause no side affects. The background worker should do its job and nothing else. Its job is not to manage the GUI. The GUI thread's job is to manage the GUI.

Think of it this way: Your car dashboard has one job: To be the GUI for your car. But if the speedometer dies you don't want that to keep the engine from starting. The speedometer and the engine are 'loosely bound' for a reason. Your program is the same. You need the GUI to run whether or not the background worker does, and you want the background worker to run regardless which GUI you have on display.

So the background worker is kept ignorant of which panel you have up. It just says "I'm 50% done". If you have the simple panel or the complex panel on display doesn't matter. It hears the event and does whatever it is designed to do to display those results. Maybe the simple panel uses a progress bar, while the advanced panel display the exact percentage. The background thread doesn't know or care how it is shown.

[immeraufdemhund]

ok. so using your advice I'm thinking that I should delete my Background worker in the main form and create *potentially* 12 background workers (1 for each panel that requires one). Loop through my controls to run the StartWorking. The StartWorking method in each panel should (if needed) start a back ground worker. The background worker in each should then have a list of things it needs to work on, then in the event have it populate the text in my label.

Did I understand correctly? Thank you very much for your great insight. I've been trying to stick to the SRP principle. I do still have bad habbits that I'm trying to break. SRP being one of them.

[tlhIn`toq]

To me that sounds like the main application is doing too much work and micromanaging the other controls. If the main application is coded for 12 background threads and panels, what happens in 6 months when you want to add a couple more panels? You have to update all your infrastructure to support them.

Personally I would make UserControls that consist of the Panel, its own backgroundworker and any pure objects that you might want to serialize for later restoration.

Now you can place 2 of your custom controls on screen... or 20... and they each take complete care of themselves. Only when one of your custom controls raises an event like LogThis(string message) or HighLevelWarning(int level) does the application have to do anything. The bulk of the work is done within the UserControl

The idea is to make a complete and fully self-contained 'Lego Block' of functionality. Then use your application to just hook them all together.



Spending a little time on these tutorials before writing any more code should help you with a better understanding of all this and enable you to architect a better blueprint for your application. Work smarter not harder.

The tutorials below walk through making an application including inheritance, custom events and custom controls, object serialization and more.
Quick and easy custom events
Bulding an application - Part 1
Building an application - Part 2
Separating data from GUI - PLUS - serializing the data to XML
WPF version (WPF-MVVM data binding)
Passing values between forms/classes
Decouple your multi-threaded work from the GUI so forms don't hang

[tlhIn`toq]

One other thing...

In each of these UserControls I would create methods for Start() and Stop(). Let the UserControl handle its own needs.

Your main application should do nothing more than:

StartAllPanels()
{
   foreach(CustomPanel cp in myPanelsCollection)
   {
      cp.Start(); // Tell the panel to start
      cp.WarningMessage += WarningHandlerMethod; // Subscribe to warning event
      cp.LogThis += LogMessageHandlerMethod; // Subscribe to logging event
   }
}

See how your main application has no idea what the panel needs to do to start up? It doesn't try to micromanage the panel and tell it to start it's backgroundworker. The main app is blissfully ignorant of the inner needs of the component. Its like a the driver of a car. The driver doesn't tell the car: Close 12v power, start fuel pump, begin ignition computer, calibrate fuel/air mix, confirm foot is on brake, start ignition... The driver just turns the key to Start(). Your application should be the same. It only needs to know what it NEEDS to know. Let the rest of the parts take on as much responsibility as you can push on them.

[immeraufdemhund]

yeah, that is what i was trying to say. It actually says this until i get all my panels converted.

            for (int i = 0; i < this.Controls.Count; i++)
            {
                if (this.Controls[i] is AbstractSuperPanel)
                {
                    var startMe = Controls[i] as AbstractSuperPanel;
                    startMe.StartWorking();
                }
            }

then it will simply just be

            for (int i = 0; i < this.Controls.Count; i++)
            {
                    var startMe = Controls[i] as AbstractSuperPanel;
                    startMe.StartWorking();
            }

[tlhIn`toq]

So you aren't adding the newly made AbstractSuperPanels to any kind of collection when you make them?

It might be fine *NOW* to just loop through all the controls on the form... But later this may cause issues. What if you add a Dashboard for a summary view... or buttons to add new panels or email results etc.?

If when you make the new superpanel you add it to a List<AbstractSuperPanel> you can then traverse that collection so easily:

foreach(var ASP in myAbstractSuperPanelsList)
{
   ASP.StartWorking();
}

Just my style and 2 cents worth.

[immeraufdemhund]

Well although you bring up a valid point, the scope of the program probably won't do that because the people i'm making this for were very specific with the look and how many panels. But then again that does seem like a good thing to incorporate for a JUST IN CASE moment.

[tlhIn`toq]

If I had a dollar for every time I heard "We'll never go beyond a need for xyz" I'd be retired on my private island.

The important thing is that it doesn't cost any more to be prepared. Just design EVERY application as if it is going to be expanded on every 6 months from some unexpected direction.

If they never add anything else: It works
If they DO add 10 other options: It doesn't break.
And it simplifies all the rest of the handling so overall product development benefits in time and maintainability.

If you don't see the application for 3 years then have to add features you aren't scratching your head and talking to yourself:

Quote

Let's see... This was a small app once, so I did it this way.
Then it grew to a medium app so I changed it to that way.
Now its going to be an intermediate app so I have to scrub all the code and do it this other way.