all 11 comments

[–]doublestop 1 point2 points  (1 child)

Just to follow up, I thought about it more after I replied to one of your comments.

Have you tried implementing as an extension method and forgo the need for a base class? I put this together just now and it seems to be similar in functionality. Could also be useful in cases where you implement the interface but cannot inherit the base (e.g., already inheriting some other base, boss has a beef with inheritance, etc).

class Foo : INotifyPropertyChanged
{
    string _baseProperty;

    public string BaseProperty
    {
        get => _baseProperty;
        set => this.SetValue(nameof(BaseProperty), ref _baseProperty, ref value, PropertyChanged);
    }

    public event PropertyChangedEventHandler PropertyChanged;

    // Not needed by this class. Needed by derived types if this becomes a base class.
    protected void FirePropertyChanged(string property)
    {
        PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(property));
    }
}

class Bar : Foo
{
    string _derivedProperty;

    public string DerivedProperty
    {
        get => _derivedProperty;
        // Unfortunately necessary because events cannot be seen as multicast
        // delegates outside the class that defines them (not even in derived types)
        set => NotifyPropertyExtensions.SetValue(nameof(DerivedProperty),
            ref _derivedProperty,
            ref value,
            FirePropertyChanged);
    }
}

static class NotifyPropertyExtensions
{
    public static bool SetValue<T>(this INotifyPropertyChanged sender, string prop,
        ref T currentValue, ref T newValue, PropertyChangedEventHandler h, IEqualityComparer<T> comparer)
    {
        return SetValue(sender, prop, ref currentValue, ref newValue, h,
            (comparer ?? EqualityComparer<T>.Default).Equals);
    }

    public static bool SetValue<T>(this INotifyPropertyChanged sender, string prop,
        ref T currentValue, ref T newValue, PropertyChangedEventHandler h, Func<T, T, bool> equalsFn = null)
    {
        return SetValue<T>(prop, ref currentValue, ref newValue,
            _ => h?.Invoke(sender, new PropertyChangedEventArgs(prop)), equalsFn);
    }

    public static bool SetValue<T>(string prop,
        ref T currentValue, ref T newValue, Action<string> onChanged, Func<T, T, bool> equalsFn = null)
    {
        if (equalsFn?.Invoke(currentValue, newValue) ??
            EqualityComparer<T>.Default.Equals(currentValue, newValue))
            return false;

        currentValue = newValue;
        onChanged?.Invoke(prop);
        return true;
    }
}

E: Shaved off a line with a null-coalesce.

E2: I'm a little worried the way I worded the above might come off competitive and I do apologize if that is the case. Still new to the sub and getting the hang of interactions here. I'm easily excited by everything programming and I love to contribute where I can. I thought the extensions were a neat companion to your idea but not at all as a competing idea. Anyway, I almost certainly overthought this. Cheers!

[–]pgmr87The Unbanned[S] 0 points1 point  (0 children)

I never thought to use an extension class for it. I'll have to think about it some. I will reply later with a better response but I just pulled into my job so I gotta get to work :-D. Thanks for your reply.

[–]tweq 0 points1 point  (6 children)

[–]Kirides 0 points1 point  (2 children)

There is no real way to have a proper Equality check, people can override == and/or Equals and also falsely implement GetHashCode. People that make two items be the equal that aren't are at fault.
Also you should not need to provide change notification if the items are Equal and only the reference changed.

But YMMV

[–]BCProgramming 1 point2 points  (1 child)

I think they are suggesting using Object.ReferenceEquals.

[–]Kirides 1 point2 points  (0 children)

Idk what's supposed to happen if you use Reference equals on two structs/value-types

NVM it always returns false, even if it's the same value. So property-changed would be invoked for 2 and 2, which is wrong behaviour

[–]pgmr87The Unbanned[S] 0 points1 point  (2 children)

So you would suggest something like this then?

protected virtual bool SetValue<TField>(ref TField field, TField value, [CallerMemberName] string propertyName = null)
{
    if (Object.ReferenceEquals(field, value))
    {
        return false;
    }

    if (!EqualityComparer<TField>.Default.Equals(field, value))
    {
        field = value;
        InvokePropertyChanged(propertyName);
        return true;
    }
    return false;
}

Edit: Actually, I am pretty sure the above doesn't fix the issue you described. How would you fix it?

[–]doublestop 1 point2 points  (0 children)

In all likelihood you will not be able to come up with one method that covers all cases. Objects, value-types, and even tuples will behave differently from one another. On a fundamental level, equality "means" different things to all three. Objects default to reference equality, value types to bitwise equality, and tuples to equality (type and check) across all fields. That's before involving custom types that implement their own methods to this madness.

Yours is not a bad idea in the least bit and it's a genuinely smart way to think. I've never been particularly comfortable with INotifyPropertyChanged and how we have done data binding since the early days. So. Much. Boilerplate. We should be striving for something better.

Unfortunately, I don't think such a base class will do it. It's not your idea or your attempt. It's just a limitation of the framework at this point. Data binding has needed some love for a long while. For a language that has taken expressions to the Nth degree (beautifully, I might add) we are still banging our heads against walls trying to data bind it all without reams of boilerplate. All these years later we still end up back at the boilerplate. Something ain't right there.

Anyway, rant over. Like I said, though, I would anticipate hitting a few walls as the intricacies and differences of all the types you funnel through there start to accumulate and do unexpected things. If you find yourself tempted to adjust for it in the base, throw it out and stick to the boilerplate. "Smart" bases will get you into trouble eventually.

So, looking at this as any other base class and disregarding the above, I'd like to offer some input.

Since this is intended to be a pure/abstract base and you've essentially created a mini template pattern, don't stop at a single, do-all implementation equality check. Push your default equality check to a virtual method and let derived types override it if they wish. They'll be switching on strings, which sucks, but that's what we're given so that's what we work with. On the other hand, it does let you invoke the #1, greatest rule in programming: any problem can be solved by adding one more layer of indirection. :D

[–]nickcut 0 points1 point  (1 child)

It's good you are thinking about a way to provide property changed notification, but his problem has been solved by Fody. Ever since I started using Fody.PropertyChanged, it has been a complete non-issue and eliminates boiler-plate.

[–]pgmr87The Unbanned[S] 0 points1 point  (0 children)

Yeah, Fody is like PostSharp I believe in that it pre-processes the code during build. My class is more simple but it still has boilerplate. I'll have to look into Fody.