Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add the equivalent of NotifyPropertyChangedFor #13

Open
paulvarache opened this issue Apr 20, 2023 · 4 comments
Open

Add the equivalent of NotifyPropertyChangedFor #13

paulvarache opened this issue Apr 20, 2023 · 4 comments

Comments

@paulvarache
Copy link
Contributor

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
@MDale-RAC
Copy link

MDale-RAC commented Jul 3, 2023

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?

@paulvarache
Copy link
Contributor Author

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: #9

@MDale-RAC
Copy link

MDale-RAC commented Jul 3, 2023

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

@paulvarache
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants