Replies: 14 comments
-
I am using PropertyChanged.Fody for this. The only thing I need to do is to install the NuGet package. It will ad code by compile time. |
Beta Was this translation helpful? Give feedback.
-
I don't like magic and do not like to have to opt out property changed. PropertyChanged should be something you explicitly say that you want and not something you would have to turn off on my opinion. I don't think it's a good way of developing view models to always add those property changes. After all, you would not always want your properties to do that. If the view only updates the view model i.e Leave this as is and let people choose to opt in autogeneration by 3rd parties. |
Beta Was this translation helpful? Give feedback.
-
You are right, you should probably have to set the attribute for each
property instead of each class.
Il giorno mer 20 mag 2020 alle 17:51 Håvard Moås <notifications@github.com>
ha scritto:
… I don't like magic and do not like to have to opt out property changed.
PropertyChanged should be something you explicitly say that you want and
not something you would have to turn off on my opinion. I don't think it's
a good way of developing view models to always add those property changes.
After all, you would not always want your properties to do that. If the
view only updates the view model i.e
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<https://github.com/dotnet/maui/issues/68#issuecomment-631772580>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AFJTYGUWK6QMDKZOXFH2QSLRSRNIHANCNFSM4NGHPF3A>
.
|
Beta Was this translation helpful? Give feedback.
-
I like the attribute way (explicitly on properties, not the whole class). If it was at the same time possible to override generated Example with a hypothetical notification manager being used to support collecting of notifications to dispatch them later etc.: public class CustomerViewModel : INotifyPropertyChanged
{
[Bindable]
public string Name { get; set; }
[Bindable]
public string Address { get; set; }
// if this method exists then it gets called on every bindable property change instead of the generated one
void OnPropertyChanged( string propertyName )
{
void NotifyPropertyChanged( string pn ) => PropertyChanged?( pn );
if( _notificationManager != null )
_notificationManager.Add( this, propertyName, NotifyPropertyChanged );
else
NotifyPropertyChanged( propertyName );
}
} |
Beta Was this translation helpful? Give feedback.
-
I think view model implementation should definitely be flexible, but would be in favour of deprecating |
Beta Was this translation helpful? Give feedback.
-
We've never been forced to use The original options if implemented would cause ALL properties to notify, which isn't always what we want, we only want properties that interact with the View to update, for this there are already options out there. Property Changed Fody, Reactive.Fody, BindableObject, Prism, MVVM Helpers, etc. |
Beta Was this translation helpful? Give feedback.
-
I'd suggest being able to use
@dhindrik As far as I'm aware |
Beta Was this translation helpful? Give feedback.
-
I'm with @yannduran - [Bindable] attribute would be awesome! then I can use automatic properties and have them be bindable - what a joy that would be! |
Beta Was this translation helpful? Give feedback.
-
PropertyChanged processes properties of classes that implement yes that may include some public properties that dont need notification, but firing a property event when nothing is using it is not a measurable performance impact in this context |
Beta Was this translation helpful? Give feedback.
-
Sorry, yes of course I meant that it produces IL for properties of classes that implement Thanks for the confirmation. |
Beta Was this translation helpful? Give feedback.
-
not sure if this interest anyone https://github.com/nicolo-ottaviani/Xamarin.BindableProperty.Fody |
Beta Was this translation helpful? Give feedback.
-
I use snippets for BindableProperty and PropertyChanged, problem solved 👍 |
Beta Was this translation helpful? Give feedback.
-
At a maximum, I'd suggest having something like Bindable as an option. Here's my concern. In large projects there are a number of cases where bindability and responsiveness are broader concerns than represented in this sample:
|
Beta Was this translation helpful? Give feedback.
-
I am against this, but if this goes anywhere the name should be [TriggersPropertyChange] which is also ugly :). Best is you do base class that wraps setting the property and triggers change event and then write snippet for that: public class ObservableObject : INotifyPropertyChanged, INotifyPropertyChanging
{
public event PropertyChangedEventHandler PropertyChanged;
public event PropertyChangingEventHandler PropertyChanging;
protected bool SetProperty<T>(ref T prop, T value, [CallerMemberName]string propertyName = "")
{
if (EqualityComparer<T>.Default.Equals(prop, value))
return false;
OnPropertyChanging(propertyName);
prop = value;
OnPropertyChanged(propertyName);
return true;
}
protected bool SetProperty<T>(object obj, string prop, T value, [CallerMemberName] string propertyName = "")
{
var propInfo = obj.GetType().GetProperty(prop);
var val = (T)propInfo.GetValue(obj);
if (EqualityComparer<T>.Default.Equals(val, value))
return false;
OnPropertyChanging(propertyName);
propInfo.SetValue(obj, value);
OnPropertyChanged(propertyName);
return true;
}
protected virtual void OnPropertyChanging([CallerMemberName]string propertyName = "")
{
PropertyChanging?.Invoke(this, new SC.PropertyChangingEventArgs(propertyName));
}
protected virtual void OnPropertyChanged([CallerMemberName]string propertyName = "")
{
PropertyChanged?.Invoke(this, new SC.PropertyChangedEventArgs(propertyName));
}
} <CodeSnippet Format="1.0.0">
<Header>
<Title>INotifyPropertyChanged Snippet</Title>
<Shortcut>propn</Shortcut>
</Header>
<Snippet>
<Declarations>
<Literal>
<ID>type</ID>
<Default>int</Default>
</Literal>
<Literal>
<ID>p</ID>
<Default>MyProperty</Default>
</Literal>
</Declarations>
<Code Language="CSharp">
<![CDATA[
$type$ _$p$;
public $type$ $p$
{
get { return _$p$; }
set { SetProperty(ref _$p$, value); }
}]]>
</Code>
</Snippet>
</CodeSnippet> |
Beta Was this translation helpful? Give feedback.
-
Summary
Creating View Models requires following the INotifyPropertyChanged interface and related patterns. This distracts the developer from doing actual business logic to wiring events and writing getters/setters, losing the benefits of automatic properties.
Doing https://docs.microsoft.com/en-us/dotnet/api/xamarin.forms.bindableobject?view=xamarin-forms is too much work compared to an automatic property.
API Changes
Add a new Attribute for the classes to be bound to UI, or perhaps extend the current BindableObject.
Option1 using an Attribute:
Option2 using an enhanced base class:
Option3 using a wrapper class:
Intended Use Case
This will help us create less complicated ViewModels and reduce coding, helping developers adhere to the mvvm
Beta Was this translation helpful? Give feedback.
All reactions