CODEWITHSUNDEEP
c#.netwinformsdata-binding
Noren
Noren

Reputation: 793

Simple-bound .NET data binding not working

Technology: .NET 4, C#, WinForms, Visual Studio 2010

I am in the processing of learning data binding and have been unable to get even a simple example to work as expected. I have a form with a label that I am binding to that shows the current mouse cursor coordinates.

public partial class Form1 : Form, INotifyPropertyChanged
{
    [Bindable(true)]
    private String cursorPosition;
    public String CursorPosition
    {
        get
        {
            return cursorPosition;
        }

        set
        {
            cursorPosition = value;
            NotifyPropertyChanged("CursorPosition");
        }
    }

    public event PropertyChangedEventHandler PropertyChanged;

    public Form1()
    {
        InitializeComponent();
    }

    private void Form1_MouseMove(object sender, MouseEventArgs e)
    {
        CursorPosition = "(" + Convert.ToString(e.X) + " , " + Convert.ToString(e.Y) + ")";
    }

    private void NotifyPropertyChanged(String propertyName)
    {
        if (PropertyChanged != null)
        {
            PropertyChanged(this, new PropertyChangedEventArgs(propertyName));
        }
    } 
}

From the designer, I have set the label's Data Binding to bind the Text property to form1BindingSource - CursorPosition. What am I missing?

Edit: Updated code snippet.

Upvotes: 2

Views: 6646

Answers (4)

stakx - no longer contributing
stakx - no longer contributing

Reputation: 84835

From the designer, I have set the label's Data Binding to bind the Text property to form1BindingSource - CursorPosition. What am I missing?

Have you set:

form1BindingSource.DataSource = this;  // (or whatever the real data source is)

e.g. in the form's constructor, after InitializeComponent?

(This assumes that your Form1 instance is the data source, and you're binding the controls to it via a BindingSource.)

A few further detail suggestions:

  1. Choosing the form itself as data source is somewhat unusual. IMHO it's better to separate all bound-to properties into a separate, non-UI data object. This then allows you to create a reusable base type for the INotifyPropertyChanged implementation.

  2. As @rfmodulator says in his answer, the BindableAttribute is attached to the field:

    [Bindable(true)]
private String cursorPosition;
public String CursorPosition
…

    You probably meant to attach it to the property:

    private String cursorPosition;
[Bindable(true)]
public String CursorPosition
…

  3. Your setter should probably look like this: 

    set
{
    if (!string.Equals(cursorPosition, value)      // +
    {                                              // +
        cursorPosition = value;
        NotifyPropertyChanged("CursorPosition");
    }                                              // +
}

    That is, only raise the PropertyChanged event when the property value actually changes.

  4. You probably want to change your NotifyPropertyChanged method to this:

    private void NotifyPropertyChanged(String propertyName)
{
    PropertyChangedEventHandler handler = PropertyChanged;          // +
    if (handler != null)                                            // ~
    {
        handler(this, new PropertyChangedEventArgs(propertyName));  // ~
    }
}

    This is because PropertyChanged could in theory change between the null check and the invocation. You can exclude this theoretical possibility by creating a local copy of the event delegate.

    P.S.: To be precise, as Jeffrey Richter points out in his book "CLR via C#", a local variable is still not quite enough: Ideally, you'd assign Interlocked.CompareExchange(ref PropertyChanged, null, null) to handler (instead of simply PropertyChanged) because using that method will prevent the JIT code generator from optimizing away the local variable (IIRC).

Upvotes: 6

rfmodulator
rfmodulator

Reputation: 3738

You're setting the Bindable attribute on the field but calling NotifyPropertyChanged with the property as the argument.

Upvotes: 1

Hasani Blackwell
Hasani Blackwell

Reputation: 2056

You shouldn't implement INotifyPropertyChanged on components, controls and forms because databinding will rely on the XXXChanged event paradigm to listen for change notification. Internally, databinding uses property descriptors to listen for change events. They hint how the class detects changes in the documentation for the PropertyDescriptor.SupportsChangeEvents property. This has to do with the history of winforms databinding. XXXChanged was the way to do databinding & change notification before .NET 2.0. INotifyPropertyChanged was introduced in 2.0 in favor of the XXXChanged pattern.

The SupportsChangeEvents property indicates whether value change notifications for this property may originate from outside the property descriptor, such as from the component itself, or whether notifications will only originate from direct calls made to the SetValue method. For example, the component may implement the INotifyPropertyChanged interface, or may have an explicit nameChanged event for this property.

Upvotes: 1

Ann L.
Ann L.

Reputation: 13975

I assume you want the PropertyChanged event to fire? You are setting the backing variable's value in Mouse_Move, not the property's value. As a result, the call to NotifyPropertyChanged won't get called.

Upvotes: 2

Related Questions