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

Warn on string.Equals() with flags in setter? #136

Open
JohanLarsson opened this issue Aug 22, 2019 · 2 comments
Open

Warn on string.Equals() with flags in setter? #136

JohanLarsson opened this issue Aug 22, 2019 · 2 comments

Comments

@JohanLarsson
Copy link
Collaborator

JohanLarsson commented Aug 22, 2019

namespace N
{
    using System;
    using System.ComponentModel;
    using System.Runtime.CompilerServices;

    public class C : INotifyPropertyChanged
    {
        private string p;

        public event PropertyChangedEventHandler PropertyChanged;

        public string P
        {
            get => this.p;
            set
            {
                if (string.Equals(value, this.p, StringComparison.Ordinal))
                {
                    return;
                }

                this.p = value;
                this.OnPropertyChanged();
            }
        }

        protected virtual void OnPropertyChanged([CallerMemberName] string propertyName = null)
        {
            this.PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
        }
    }
}

Above is not very nice as the expectation is that the property gets the assigned value. /cc @jnm2

@jnm2
Copy link
Collaborator

jnm2 commented Aug 22, 2019

I agree; even if the notification is skipped, setting should not be skipped unless you can prove that there would be no way to observe the difference.

@JohanLarsson
Copy link
Collaborator Author

Ok, adding analyzer then, can always be disabled. Has chances of catching bugs.

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

No branches or pull requests

2 participants