4

I've inherited a Silverlight project with dubious code quality overall, and there is one construct that I'm not sure whether I should touch it:

public SomeClass Self
{
    get
    {
        return this;
    }
}

It is used in XAML Bindings, with parameters, sometimes complex like this:

Visibility="{Binding Self, ConverterParameter=!, Converter={StaticResource SmartAssConverter}}"

And it is used in a PropertyChanged notification (MVVM Light):

RaisePropertyChanged("Self");

So, is there something preventing me from just doing this:

Visibility="{Binding ConverterParameter=!, Converter={StaticResource SmartAssConverter}}"

which, I've tested, still shows just fine?

Rephrasing my question, is the necessity to 'raise property changed' forcing this kind of (IMHO ugly) construct?

Edit: rephrasing again, is there a more elegant solution to notify binded controls that their target has changed, or should I look into reworking the Converters?

jv42
  • 8,521
  • 5
  • 40
  • 64

3 Answers3

2

What if the object (i.e. Self) changes? When using the Self property you can leverage the INotifyPropertyChanged interface to tell the binding to updated. If you remove the property, then how would you update?

You can try doing RaisePropertyChanged(string.Empty), but I don't think that would work.

Generally the converters would just be passed the properties they need, not the entire object. But in Silverlight, there is no MultiBinding so you are limited to a single property.

You can either add a new property to your object that performs the same operation as the converter, or wrap it with another object that adds the property. This is generally the role of a view-model.

CodeNaked
  • 40,753
  • 6
  • 122
  • 148
  • Mmmm, I think you nailed it with 'MultiBinding'. Some of these converters depend on several properties. I think too much is done in converters too, so I think you're right too with the 'move it to the ViewModel' part. – jv42 Aug 02 '11 at 15:10
  • 2
    If you do want to go the multibinding route, I have created a SL multibinding solution here: http://www.scottlogic.co.uk/blog/colin/2010/05/silverlight-multibinding-solution-for-silverlight-4/ – ColinE Aug 02 '11 at 15:16
  • 1
    I just re-read a Converter's code, where I added this comment a while ago: '// TODO: (J) replace by property in ViewModel'. Guess I still agree with myself then :) – jv42 Aug 02 '11 at 15:43
  • Using '{Binding Self}' and '{Binding}' is the same - both point to the underlying view model. When the view model is replaced the data context of the bound UI element is changed and all bindings are refreshed. If you want to notify the view that **all** properties of the view model have changed (or if you want to force a refresh) use `RaisePropertyChanged(null)`. – AxelEckenberger Aug 03 '11 at 09:50
  • 1
    @Obalix - It's only the same if the DataContext changes, which may not be the case (if the object in question is the root view-model/data context). Also INotifyPropertyChanged only notifies that a property or all properties on an object changed, not that the object itself changed. So INotifyPropertyChanged cannot be used to update a `{Binding}`, where the object being bound to is the one who is firing the PropertyChanged event. – CodeNaked Aug 03 '11 at 11:32
  • @CodeNaked, sorry to disagree! For the equality issue: The `Self` property returns this, therefore, the property and the instance change at the same time. From this reasoning it is the same to use `{Binding}` (returning the object assigned to the DataContext) or `{Binding Self}` (returning the Self property of object assinged to the DataContext, which in turn returns the object itelf). – AxelEckenberger Aug 03 '11 at 14:29
  • @CodeNaked: For the notification issue: AFAIK, if the data context changes all bindings on the subordinate elements are refreshed. Therefore, there is no need to notify the bindings that the `DataContext` has changed. So, acually you do not need any `INotifyPropertyChanged` at all. But, if you want to notify that all properties of an object used as a `DataContext` should be rebound you can use `RaisNotifyPropertyChanged(null)`. – AxelEckenberger Aug 03 '11 at 14:33
  • @Obalix - Your first statement is correct. The issue is with notifying of changes, where you are assuming the DataContext is changed. If you have `window.DataContext = someObject;`, then there is no way to know to update the DataContext based on someObject alone. If you have `DataContext={Binding SomeObject}`, then again from SomeObject you cannot update the DataContext, only the object that has the SomeObject property can do that. Using the `Self` property allows SomeObject to update the binding to itself. – CodeNaked Aug 03 '11 at 15:02
1

I typically implement the IChangeTracking and INotifyPropertyChanged interfaces, and then do the following in the default constructor:

public SomeClass()
{
    this.PropertyChanged += new PropertyChangedEventHandler(OnNotifiedOfPropertyChanged);
}

private void OnNotifiedOfPropertyChanged(object sender, PropertyChangedEventArgs e)
{
    if (e != null && !String.Equals(e.PropertyName, "IsChanged", StringComparison.Ordinal))
    {
        this.IsChanged = true;
    }
}

The IsChanged property raises property change notifications, and so you can bind to IsChanged to be notified when the class has been modified without needing to expose the class itself as a 'Self' property.

jv42
  • 8,521
  • 5
  • 40
  • 64
Oppositional
  • 11,141
  • 6
  • 50
  • 63
1

The code you have shown is a little clumsy looking, but I don't think it is so bad that it need to be reworked. You are right, you can remove the path altogether and this will work for a one-time evaluation of the binding. However, the worrying part is where the code raises a property change for 'Self' in order to re-evaluate the bindings (great hack ... I'll remember that for future use!)

The correct approach here is to change the DataContext itself, this is effectively a change of 'self' and will cause all binding to be re-evaluated.

Personally, I would not spend too long on this, I have seen much much worse!

ColinE
  • 68,894
  • 15
  • 164
  • 232
  • Ok, thanks for your opinion! I'm not so much worried about updating this project (which I hope will be wholly re-done, for much worse smells than that), as wanting to find the 'correct' approach for new code I'll be writing. – jv42 Aug 02 '11 at 15:12