33

I'm refactoring some code.

Right now there are quite a few places with functions like this:

string error;
if (a) {
   error = f1(a, long, parameter, list);
}
else {
   error = f2(the_same, long, parameter, list);
}

before refactoring f1 and f2 (which are large, but do similar things), I'd like to refactor to:

string error = (a ? f1 : f2)(a, long, parameter, list);

As one would do in C. (The function signatures are identical)

But I get an error:

"Error 13 Type of conditional expression cannot be determined because there is no implicit conversion between 'method group' and 'method group'"

This would allow me to recognize that the parameter lists are identical by the initial refactoring giving invariant behavior, and also refactor the calls in a single place, ensuring that all during these various refactorings, nothing gets broken as I change the calling interface to the method.

Am I missing something small which would allow a syntax close to this to work (as opposed to a whole bunch of extra delegate type definitions etc)?

Sorry to edit, but there is actually a return value, and yes, unfortunately, it is a string. ;-(

Right now, I'm settling for this:

string error = a ? f1(a, long, parameter, list) : f2(a, long, parameter, list);

The problem is that the parameter list are indeed very long, and are going to get refactored, and I'd prefer to have them consolidated first and deal with compiler errors as I change them.

Cade Roux
  • 88,164
  • 40
  • 182
  • 265

7 Answers7

32

For the ? to work the compiler needs an explicit type for at least one of the operands. You can provide one here via a cast operator

(a ? (Action<T1,T2,T3,T4>)f1 : f2)(a, long, parameter, list);

Replace T* with the actual types of the delegate parameters

JaredPar
  • 733,204
  • 149
  • 1,241
  • 1,454
9

You'll have to instantiate one of the methods as a specific compatible delegate type. There's just no way around it. This will, unfortunately, be a little more verbose than you're looking for:

(a ? new Action<T1, T2, T3, T4>(f1) : f2)(a, long, parameter, list);

You're going to have to make the parameters involved explicit, whether that's by using an Action (or Func) generic overload that fits or declaring your own delegate type.

This comes down to type resolution. Given the expression:

condition ? tVal : fVal

The compiler doesn't look for common assignment-compatible ancestors for tVal and fVal (and, even if it did, the litany of different delegate types to which each might be valid could be huge); if there isn't assignment compatibility between the types of tVal and fVal in either direction, the compiler makes you be explicit as to what you want.

For what it's worth, you should be aware that taking this approach will allocate a new delegate to either f1 or f2 every time this method is called, then that delegate will be invoked, then discarded. I bring this up only because delegate invocation is slower than ordinary early-bound (or even virtual) method invocation. It may well not be a consideration and the tradeoff might be worth it, but it's still worth knowing.

Adam Robinson
  • 182,639
  • 35
  • 285
  • 343
  • Casting is sufficient here. There's no need to allocate a new delegate – JaredPar Mar 03 '11 at 20:58
  • @JaredPar: Delegate casting (and type inference) is just syntactic sugar; you'll still end up with a new delegate instance. In this circumstance, it casting wouldn't be any less verbose than the traditional syntax. – Adam Robinson Mar 03 '11 at 21:00
  • @Adam, true. Forgot that there's no difference there. – JaredPar Mar 03 '11 at 21:01
  • Is the "new" really needed? The answer from JaredPar seems to work without a "new". – rskar Mar 03 '11 at 21:21
  • @rskar, they produce identical code. That's the discussion @Adam and I've been having. Casting a method group to a delegate and explicitly creating the delegate end up producing the same code (creation of the delegate) – JaredPar Mar 03 '11 at 21:23
  • @JaredPar - So, if the results are exactly the same, is it safe to save ourselves from typing the apparently superfluous "new"? Or is this a matter of "code smell"? – rskar Mar 03 '11 at 21:29
  • @rskar they produce identical IL for this scenario so one is as safe as the other. – JaredPar Mar 03 '11 at 21:30
8

If you specify a delegate type, it will allow you to do what you're asking:

    (test ? (Action<int,int>)M1 : M2)(10, 15)

With the declarations:

    void M1(int a, int b)
    {
    }

    void M2(int a, int b)
    {
    }

Tested with .Net 4, but should apply for .Net 3.5

Matt DeKrey
  • 11,582
  • 5
  • 54
  • 69
7

You can do that by declaring a delegate, as you pointed out.

I notice that you wrote that you are doing this in quite a few places. Another alternative that might be more suitable is to use interfaces. Instantiate one of two different types depending on the value of a, then call the method on that object.

IFoo foo = a ? new Foo1() : new Foo2();
foo.f(a, long, parameter, list);

If you have multiple methods that need to change simultaneously depending on the value of a then you can include them all in the same interface and you will only need to test a once.

Mark Byers
  • 811,555
  • 193
  • 1,581
  • 1,452
  • 1
    Adding link to longer discussion of this approach for future refrence - http://stackoverflow.com/questions/410532/whats-the-best-alternative-to-an-out-of-control-switch-statement – Alexei Levenkov Mar 03 '11 at 21:23
2

No, basically, without making it less efficient. If there is a return value, you can use:

var result = cond ? methodA(a,b,c,d) : methodB(a,b,c,d);

but that is about it.

Well, you can create a pair of delegates from the method group, but that adds overhead for no good reason. I won't endorse it.

Marc Gravell
  • 1,026,079
  • 266
  • 2,566
  • 2,900
  • "but that adds overhead..." Looks to me we should profile that. I'm guessing the impact is minimal at worst. – rskar Mar 03 '11 at 21:18
  • @rskar I think we both know writing the test is going to take more time than this will *ever* save, but it is still an unnecessary *code* reduction... I guess I just don't see the point of doing this. – Marc Gravell Mar 03 '11 at 21:32
1

I would think something like the following would work better:

a ? f1(a, long, parameter, list) : f2(a, long, parameter, list);

apaq11
  • 82
  • 2
  • I think he's trying to avoid duplicating the code that pushes the same set of arguments, such as one could do with C. I doubt that there's any loss of computing speed trying to do that. – rskar Mar 03 '11 at 21:17
0

Cast the result of the ternary to an Action

((Action<int, int>)(a ? f1 : f2))(int1, int2);
Bob
  • 97,670
  • 29
  • 122
  • 130
  • Sorry, but that syntax didn't work with VS2008. Had to go with (a ? (Action) f1 : f2)(1, null, 2.0f); – rskar Mar 03 '11 at 21:15
  • @rskar I think you might be off a parentheses or two. – Bob Mar 03 '11 at 22:15
  • Tried again, but no joy. I doubt it's a parentheses thing. Literally, I copied and pasted from the above. – rskar Mar 04 '11 at 20:59