Best practice with Winforms that are externally updated

Question

I am wondering whether it is a good idea to make labels public so other classes can change them and get their value. Is this a good idea? If not, how should it be done then?

Answer 1

I wouldn't make the label public.

It would be better to add a public method that was specific to what the label was displaying, and have it update the label.

For example, if your label was a "System status" label, you might want to add (to your Form/UserControl):

public void SetStatusInformation(string currentStatus)
{
     this.statusLabel.Text = currentStatus;
}

This allows you, later, to change how this information is displayed (in case you want to use a different control), and also simplifies your API, since the public methods are very clear to the user.

Answer 2

it's a bad idea. WinForms leaves many "what's the best way to do X?" questions open; and your best answer is to follow established patterns and practices (which aren't WinForms specific).

Read up on the MVP or MVC patterns. They are both high-level patterns which focus on seperating out your UI-specific code from your business-logic. Without this seperation your application can quickly become a maintenance nightmare, and things that should be simple get much more complicated.

For your specific scenario you would likely end up with a Model (your business-logic) which uses databinding to show it's data on the WinForms screen. When a change on the UI occurs it would be the Model that receives the change, and that change would propagate to the UI via databinding.

Answer 3

I would suggest wrapping in a setter property or method because it's very possible you'll have to do something like add logging or re-call on window's main thread if the caller is from another one. I found it easier to just always use code like the following when exposing functionality that lets clients update anything graphical.

public void SetStart()
    {
        if (this.InvokeRequired)
        {
            this.Invoke((MethodInvoker)delegate()
            {
                this.SetStart();
            });
        }
        else
        {
            progressBar1.Value = 0;
            progressBar1.Visible = true;
        }
    }