current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 4 down vote favorite

In my current project, I find myself using Dispatcher.Invoke a lot. Before each call I test if the current dispatcher is the right one. I want to write a wrapper class like so:

class SafeRunner
{
    //From the thread we always need to run on
    public static System.Windows.Threading.Dispatcher disp;

    public static void Run<T>(Action<T> f, T arg)
    {
        if (disp != System.Windows.Threading.Dispatcher.CurrentDispatcher)
        {
            disp.Invoke(f, arg);
        }
        else
        {
            f(arg);
        }
    }
    public static void Run<T1, T2>(Action<T1,T2> f, T1 arg1, T2 arg2)
    {
        if (disp != System.Windows.Threading.Dispatcher.CurrentDispatcher)
        {
            disp.Invoke(f, arg1, arg2);
        }
        else
        {
            f(arg1, arg2);
        }
    }
    public static void Run<T1, T2, T3>(Action<T1,T2,T3> f, T1 arg1, T2 arg2, T3 arg3)
        {...}
    public static void Run<T1, T2, T3, T4>(Action<T1,T2,T3,T4> f, T1 arg1, T2 arg2, T3 arg3, T4 arg4)
        {...}
}

And I can use it like this:

SafeRunner.Run<string,string>((a,b) => {
    doSomething(a,b);
    doSomethingElse(a);
    // Maybe a few more lines here
}, "Hello", "World");

This works, but doesn't feel right. Is there a better way?

share|improve this question
    
Your invocation may be able to be written as SafeRunner.Run(doSomething, "Hello", "World"). The methodgroup/delegate should match the parameters already, after all. That makes it closer to the dispatcher's syntax too, which is probably a good thing. –  Magus Apr 1 '14 at 18:50
    
@Magus Thanks. I was not clear in that there may be multiple lines of code. I have updated my post. –  Johnny Mopp Apr 1 '14 at 18:54

2 Answers 2

up vote 2 down vote

A couple of things I notice, and they are both around the if statements

I would flip the if statements around so that they are checking the positive test. I would also put the disp on the right side of the ==. You can eliminate the else statement, all it really does is clutters up the code.

if (System.Windows.Threading.Dispatcher.CurrentDispatcher == disp)
{
    f(arg);
    return;   
}

disp.Invoke(f, arg);
share|improve this answer
    
Without the else, when CurrentDispatcher == disp the function will get run 2x. I only want it to run once. –  Johnny Mopp Apr 1 '14 at 18:56
    
Good point, for some reason I had it my head it was returning :) You can just add a return into the if... –  Jeff Vanzella Apr 1 '14 at 19:05
up vote 1 down vote

I suggest that you call your class SafeDispatcher, and your methods Invoke, since you actually wrap a Dispatcher (not Runner), and Invoke (not Run which does something completely different in Dispatcher).

Also, if performance is not an issue, you can use DynamicInvoke, and implement the whole thing in a single method:

class SafeDispatcher
{
    public static System.Windows.Threading.Dispatcher disp;

    public static void Dispatch(Delegate f, params Object[] args)
    { 
        if (System.Windows.Threading.Dispatcher.CurrentDispatcher == disp)
        {
            f.DynamicInvoke(args);
        }
        else
        {
            disp.Invoke(f, args);
        }
    }
}
share|improve this answer
    
So, to call: SafeDispatcher.Dispatch(new Action<string>((s)=>{...}), "hello");, correct? –  Johnny Mopp Apr 10 '14 at 12:27
    
I believe that SafeDispatcher.Dispatch((s)=>{...}, "hello"); should also work –  Uri Agassi Apr 10 '14 at 12:30

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.