Overriding methods with actions as parameters

Question

I have a set of methods that allows users to easily user the PropertHasChanged event and allows then to do some extra processing. Here is the method:

public virtual void SetPropertyValue<T>(ref T currentValue, T newValue, Action<T> extraFunction = null, Action voidAfterSetAction = null) where T : class
        {
            if (currentValue == newValue) return;

            currentValue = newValue;

            PropertyHasChanged();

            if (extraFunction != null) extraFunction(newValue);

            if (voidAfterSetAction != null) voidAfterSetAction();
        }

It has become apparent to me that I would sometimes need the old value in the extraFunction action. This is how I intended to do that:

public virtual void SetPropertyValue<T>(ref T currentValue, T newValue, Action<T, T> extraFunction = null, Action voidAfterSetAction = null) where T : class
        {
            var oldVal = currentValue;

            if (currentValue == newValue) return;

            currentValue = newValue;

            PropertyHasChanged();

            if (extraFunction != null) extraFunction(oldVal, newValue);

            if (voidAfterSetAction != null) voidAfterSetAction();
        }

As you may notice, the extraFunction action now takes two parameters. VS didnt have an issue with me creating the method (no red qwigglies) but when I build it throws MANY errors that say that the usage between the first method and the second method is ambiguous. If that is the case then how can I achieve what I am looking for?

EDIT

Here is the usual usage of the method:

SetPropertyValue(ref _streetAddress1, value, null, () => SalesData.StreetAddress1 = value);

As Jon asks below: please show us the actual call sites, since that's what is causing the problem. You have "MANY" errors because you call the method "MANY" times! — dlev, Feb 29 '12 at 18:50
I updated. I think that Jon's advice is sound (of course). I'm just going to change its name. — The Sheek Geek, Feb 29 '12 at 18:59
@TheSheekGeek In my (potentially incorrect) opinion, It's almost never a good idea to use optional parameters on any thing other then the built in "primitive" types. — asawyer, Feb 29 '12 at 19:02
I am not sure about this. I, personally, have not heard that. Unfortunately, I'm stuck because it has been so ingrained in the system and is before my time here. There would be A LOT of code changes required. — The Sheek Geek, Feb 29 '12 at 19:05
@TheSheekGeek And there's your answer: If you have `null` for the third parameter, how can overload resolution choose between the version with the `Action` parameter and the version with the `Action` parameter? They're equally good! As you said, Jon's advice is sound: ditch one of the overloads. — dlev, Feb 29 '12 at 19:08
You are so right!!! I (obviously) did not think of that. When developers lose their brain you guys are always there to help. Good show! — The Sheek Geek, Feb 29 '12 at 19:15
@TheSheekGeek It's not obvious at first that optional parameters are 'bound' if you will at the call site, thats all. — asawyer, Feb 29 '12 at 19:36

score 3 · Accepted Answer · answered Feb 29 '12 at 18:32

3

Firstly, this is not overriding - this is overloading.

It's fine from the point of view of the method declarations - I suspect it's the call sites which are ambiguous. Unfortunately you haven't shown us any of those.

Personally I would use two different names here to keep things simpler. Overloading can be a complex business, and by the time you've got delegates involved (with anonymous functions, method group conversions etc) it's even worse than normal - and optional parameters add to the complexity too! Having methods with different names keeps things much clearer.

Alternatively, do you need to overload it at all? Can't you just have the version with the Action<T, T> and just ignore the "old" value within the callback when you don't care about it? That would simplify things.

answered Feb 29 '12 at 18:32

Jon Skeet

1,421,763
867
9,128
9,194

1

I imagine it's because of the =null default values on the action parameters. They are set at the call site, so it is potentially ambiguous. – asawyer Feb 29 '12 at 18:36
@asawyer: Could be. It's hard to say without seeing any call sites. The mixture of generics, delegates and optional parameters when overloading is a pretty nasty one. – Jon Skeet Feb 29 '12 at 18:37
Im sorry, I meant overloading and my fingers got away from me. I've been typing it too much today. – The Sheek Geek Feb 29 '12 at 19:03

Overriding methods with actions as parameters

1 Answers1