Combining Action and Func in one parameter

Question

I have many methods that require some logging with the same pattern. Some methods need to return some value, some don't. I have created a method with Action parameter to avoid copypasting all of the logic. It looks like this:

private void Execute(Action action)
{
   Logger.Start();
   try
   {
      action();
   }
   catch(Exception exception)
   {
      Logger.WriteException();
      throw;
   }
   finally
   {
       Logger.Finish();
   }
}

Now I have some calls that like that

public void DoSomething(string parameter)
{
    Execute(() => GetProvider(parameter).DoSomething());
}

But I need some function that return values. What are the best ways to do it? I have found two now:

1) Create a copy of Execute method with Func

private T Execute<T>(Func<T> action)
{
   Logger.Start();
   try
   {
      return action();
   }
   catch(Exception exception)
   {
      Logger.WriteException();
      throw;
   }
   finally
   {
       Logger.Finish();
   }
}

This method works but has some copy paste as well.

2) Trick the parameter into being an Action:

public Result DoSomething(string parameter)
{
    Result result = null;
    Execute(() => result = GetProvider(parameter).DoSomething());
    return result;
}

This does not require copy paste but does not look so nice.

Is there a way to join Action and Func somehow to avoid any of these methods or may be there is another way to achieve the same result?

I use the second of your approaches. I couldn't find a nice way to do it any other way, so I'll be interested to see any answers to your question! — Matthew Watson, Apr 02 '13 at 13:36
It seems to be related: http://stackoverflow.com/q/4279210/55209 — Artem Koshelev, Apr 02 '13 at 13:39
consider using aspects http://stackoverflow.com/questions/1416880/aspect-oriented-programming-in-c-sharp — Lanorkin, Apr 02 '13 at 13:51
Can you put this pattern (Logger.Start, try/catch/finally, Logger.WriteException, Logger.Finish) into your `Logger` class itself? At that point, the amount of code duplication is fairly minimal in general, enough that wrapping the `Func` as an `Action` with closure semantics isn't really necessary. I suspect you'd want your logging to has as minimal a processing overhead as possible. (or, who cares? Premature optimization!) But I would definitely consider throwing the `Execute` overloads into `Logger` itself. EDIT: I guess `Logger` isn't threadsafe, but that isn't a concern, eh? — Chris Sinclair, Apr 02 '13 at 13:58

score 7 · Accepted Answer · answered Apr 02 '13 at 13:36

7

A third option is to still overload Execute, but make the Action version work in terms of the Func version:

private void Execute(Action action)
{
    // We just ignore the return value here
    Execute(() => { 
        action();
        return 0; 
    });
}

Of course all this would be simpler if void were more like a "real" type (like Unit in F# et al), at which point we could just have Task<T> instead of Task and Task<T> as well...

answered Apr 02 '13 at 13:36

Jon Skeet

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

I thought of this method as well but did not even include it as it looks quite dirty to me as you are pretending to be a Func by using int for a generic type and returning default value that is never used. – Ilya Chernomordik Apr 02 '13 at 13:38
1

@IlyaChernomordik: I think that's cleaner than assigning to a local variable within an `Action` to make it act like a `Func`, personally. But the point is that it's within `Execute`, not within `DoSomething` - you only need this "dirt" in a single place, not in multiple callers. – Jon Skeet Apr 02 '13 at 13:40
@ JonSkeet May be you are right and it's less dirty. Or may be the same dirty but in one place :) – Ilya Chernomordik Apr 02 '13 at 13:44

p.s.w.g · Answer 2 · 2013-04-02T14:50:59.943

3

Create a copy of Execute that converts the Func into an Action. You only have to write that ugly code once, and you don't end up with a complete second copy of the Execute method:

private T Execute<T>(Func<T> func)
{
    T result = default(T);
    this.Execute(() => { result = func(); });
    return result;
}

...

public Result DoSomething(string parameter)
{
    return Execute(() => GetProvider(parameter).DoSomething());
}

edited Apr 02 '13 at 14:50

answered Apr 02 '13 at 13:40

p.s.w.g

146,324
30
291
331

@ p.s.w.g The problem with this approach is that I have both value types and classes as return. And your code won't compile because local variable is not initialized before return. – Ilya Chernomordik Apr 02 '13 at 13:43
@IlyaChernomordik Just initialize `result` to `default(T)`. See my updated answer. – p.s.w.g Apr 02 '13 at 13:45
@ p.s.w.g Right, didn't think about default one. Actually this update is pretty logical for my second method, don't know why I have not though about it. – Ilya Chernomordik Apr 02 '13 at 13:58
@ p.s.w.g By the way return should be before Execute it seems in your example (DoSomething function) – Ilya Chernomordik Apr 02 '13 at 14:05
@IlyaChernomordik oops you're right. Yes `default(T)` is a very handy little trick. – p.s.w.g Apr 02 '13 at 14:11
@ p.s.w.g By the way, for some reason compiler thinks that this is a recursive call so I had to cast the ()=>result=func() to Action. It does not return a value, so I don't know how it thinks it's func... – Ilya Chernomordik Apr 02 '13 at 14:12
May be Jon Skeet is right and it's better to pretend that you are Func and not use the return value than to pretend that you are Action and use additional variable. – Ilya Chernomordik Apr 02 '13 at 14:19
@IlyaChernomordik The assignment operator returns the value assigned, so `i = 5` is five, not `void`. In your case, `result = func()`, when evaluated as an expression, resolves to whatever `func()` resolves to, not `void`, which is why the lambda has a return value. – Servy Apr 02 '13 at 14:28
@Servy how's that? (see my updated answer). Seems slightly nicer than casting to an `Action` – p.s.w.g Apr 02 '13 at 14:51
@p.s.w.g Sure, fixing it isn't much of an issue either way, I was just explaining the behavior. – Servy Apr 02 '13 at 14:52

score 2 · Answer 3 · answered Apr 02 '13 at 14:35

Here's another option. Instead of having the logging framework call your actual code, have your actual code call the logging framework. Something like this would do the trick (greatly simplified).

public class LoggerScope : IDisposable {

    private bool disposed;

    public LoggerScope() {
        Logger.Start();
    }

    public void Dispose() {
        if(!disposed) {
            Logger.Finish();
            disposed = true;
        }
    }
}

Used as follows:

        using(var scope = new LoggerScope()) {
            // actual code goes here
        }

Handle exceptions separately by catching and logging them only once at the top level of your code.

Advantages:

Avoids the need for lambdas all over the place, so the exception stack trace is a bit cleaner.
You can add arbitrary contextual data to the LoggerScope class, e.g. GUID, timestamp, logical task description text.

The disadvantage is that you have to create a special class for each special case. Though I have used this approach for doing reader writer lock. — Ilya Chernomordik, Apr 02 '13 at 14:50

Combining Action and Func in one parameter

3 Answers3