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

Provide IsNotBusy() method or IsNotBusy readonly property with a new attribute [DependsOnProperty(nameof(IsBusy))] #938

Closed
suugbut opened this issue Sep 6, 2024 · 1 comment
Labels
feature request 📬 A request for new changes to improve functionality

Comments

@suugbut
Copy link

suugbut commented Sep 6, 2024

In my opinion ObservableProperty (as a publisher) should not know anything about ICommand or other properties (as a subscriber). So the following

public partial class PersonViewModel : ObservableObject
{
    [ObservableProperty]
    [NotifyCanExecuteChangedFor(nameof(SaveCommand))]
    [NotifyPropertyChangedFor(nameof(IsNotBusy))]
    private boo isBusy;

    [RelayCommand(CanExecute = nameof(IsNotBusy))]
    private void Save() { }
    
    private bool IsNotBusy => !IsBusy; // it can be a method too
}

is more elegant if it can be simplified as follows.

public partial class PersonViewModel : ObservableObject
{
    [ObservableProperty]
    private bool isBusy;

    [RelayCommand(CanExecute = nameof(IsNotBusy))]
    private void Save() { }
    
    [DependsOnProperty(nameof(IsBusy))] // <======= the requested attribute.
    private bool IsNotBusy => !IsBusy;
}

When SaveCommand depends on IsNotBusy that depends on IsBusy, it is enough to only decorate IsNotBusy with DependsOnProperty(nameof(IsBusy)).

Interestingly DependsOnProperty lets CanSave(), for example, depend on property IsBusy defined in the base class BaseViewModel.

public partial class BaseViewModel : ObservableObject
{
    [ObservableProperty]
    private bool isBusy;
}
public partial class PersonViewModel : BaseViewModel
{
    [ObservableProperty]
    private bool isDeleted;

    [RelayCommand(CanExecute = nameof(CanSave))]
    private void Save() { }
    
    [DependsOnProperty(nameof(IsBusy))] // <======= the requested attribute.
    [DependsOnProperty(nameof(IsDeleted))] // <======= the requested attribute.
    private bool CanSave => !IsBusy && !IsDeleted;
}

Any comments are welcome.

@suugbut suugbut added the feature request 📬 A request for new changes to improve functionality label Sep 6, 2024
@suugbut suugbut changed the title Provide CanExecute() method with a new attribute [CanExecuteDependsOnProperty(nameof(IsBusy))] Provide IsNotBusy() method with a new attribute [CanExecuteDependsOnProperty(nameof(IsBusy))] Sep 6, 2024
@suugbut suugbut changed the title Provide IsNotBusy() method with a new attribute [CanExecuteDependsOnProperty(nameof(IsBusy))] Provide IsNotBusy() with a new attribute [CanExecuteDependsOnProperty(nameof(IsBusy))] Sep 6, 2024
@suugbut suugbut changed the title Provide IsNotBusy() with a new attribute [CanExecuteDependsOnProperty(nameof(IsBusy))] Provide IsNotBusy() method or IsNotBusy readonly property with a new attribute [DependsOnProperty(nameof(IsBusy))] Sep 6, 2024
@hawkerm
Copy link

hawkerm commented Sep 6, 2024

Duplicate of #377 and #367.

A [DependsOn] was already discussed in the past, this comment touches on it here: #367 (comment); these parts in particular are relevant:

It'd introduce a different way to do exactly the same, which adds further confusion.
It breaks the established pattern of generators immediately affecting annotating code, not indirectly.
It makes the generators less efficient, as they'd now have to have extra logic to look at all annotated properties.

@suugbut suugbut closed this as completed Sep 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request 📬 A request for new changes to improve functionality
Projects
None yet
Development

No branches or pull requests

2 participants