maui-bindableproperty-generator icon indicating copy to clipboard operation
maui-bindableproperty-generator copied to clipboard

Add the equivalent of `NotifyPropertyChangedFor`

Open paulvarache opened this issue 1 year ago • 4 comments

I find myself quite often having to use the OnChanged property of the AutoBindable attribute to call OnPropertyChanged on a property with a getter. Here is an example:

[AutoBindable(OnChanged = nameof(OnFirstNameChanged))]
readonly string _firstName;

[AutoBindable(OnChanged = nameof(OnLastNameChanged))]
readonly string _lastName;

public string FullName => $"{FirstName} {LastName}";

void OnFirstNameChanged()
{
	OnPropertyChanged(nameof(FullName));
}

void OnLastNameChanged()
{
    OnPropertyChanged(nameof(FullName));
}

I have a computed property relying on 2 other bindable properties, which I believe is a common pattern.

Here is what I would suggest having:

[AutoBindable]
[NotifyPropertyChangedFor(nameof(FullName))]
readonly string _firstName;

[AutoBindable]
[NotifyPropertyChangedFor(nameof(FullName))]
readonly string _lastName;

public string FullName => $"{FirstName} {LastName}";

Pros:

  • More declarative description of dependency between bindable properties and computed properties
  • Less code to reflect changes across properties
  • More in line with the MVVM community toolkit

Cons:

  • None I can think of

paulvarache avatar Apr 20 '23 09:04 paulvarache

readonly string _firstName; [..] readonly string _lastName;

I'm confused by the usage of readonly in this context, how would the values (of FirstName or LastName or the backing fields) change in this context?

MDale-RAC avatar Jul 03 '23 14:07 MDale-RAC

The fields declared here are never used. [AutoBindable] simply reads the names of the fields, remove any _ and transform to PascalCase, then generate a BindableProperty. The BindableProperty is the holder of the value, and accessors are generated to access its value.

Check out the example here: https://github.com/rrmanzano/maui-bindableproperty-generator#usage---simple-implementation

_placeholder is declared, but in the generated code it never appears once.

Adding readonly seems to be a pattern here. I am not 100% sure but I suspect it helps the compiler remove the field at build time, since it is never used.

This issue goes over the subject and proposes an alternative: https://github.com/rrmanzano/maui-bindableproperty-generator/issues/9

paulvarache avatar Jul 03 '23 14:07 paulvarache

First off, thanks for the detailed explanation, I appreciate it! :)

The fields declared here are never used.

So the AutoBindable attribute then differs in this regard from .NET MAUI MVVM and its ObservableProperty?

For instance:

public partial class Test
{ 
        [ObservableProperty]
        private bool _isBusy;
}

Generates:

using ObservablePropertyAttribute = CommunityToolkit.Mvvm.ComponentModel.ObservablePropertyAttribute;

partial class Test
{
        /// <inheritdoc cref="_isBusy"/>
        [global::System.CodeDom.Compiler.GeneratedCode("CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator", "8.2.0.0")]
        [global::System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage]
        public bool IsBusy
        {
            get => _isBusy;
            set
            {
                if (!global::System.Collections.Generic.EqualityComparer<bool>.Default.Equals(_isBusy, value))
                {
                    OnIsBusyChanging(value);
                    OnIsBusyChanging(default, value);
                    OnPropertyChanging(global::CommunityToolkit.Mvvm.ComponentModel.__Internals.__KnownINotifyPropertyChangingArgs.IsBusy);
                    _isBusy = value;
                    OnIsBusyChanged(value);
                    OnIsBusyChanged(default, value);
                    OnPropertyChanged(global::CommunityToolkit.Mvvm.ComponentModel.__Internals.__KnownINotifyPropertyChangedArgs.IsBusy);
                }
            }
        }
}

If I understand correctly, whereas .NET MAUI MVVM ObservableProperty's use the backing field, AutoBindable does not?

(Sorry if this is off-topic, I was intrigued by the use of readonly and somewhat confused)

EDIT: D'OH, it's a different paradigm, ContentPage vs ContentView... Haha, uups

MDale-RAC avatar Jul 03 '23 14:07 MDale-RAC

That's correct, [AutoBindable] doesn't use a backing field, it is a very convenient way to declare a BindableProperty and associated accessors without the boilerplate code

paulvarache avatar Jul 03 '23 14:07 paulvarache