improve code for delayed dispatcher invoke

Question

I knocked this up quickly to allow delayed Invoke on the dispacther, along with canceling the existing invocation and replacing with a new one, this allows throttling an action.

public static class DispatcherExtensions
{
    private static Dictionary<string, DispatcherTimer> timers = new Dictionary<string, DispatcherTimer>();
    private static object syncRoot = new object();

    public static void DelayInvoke(this Dispatcher dispatcher, string namedInvocation, 
        Action action, TimeSpan delay, DispatcherPriority priority = DispatcherPriority.Normal)
    {
        lock (syncRoot)
        {
            DispatcherTimer timer = null;
            if (timers.TryGetValue(namedInvocation, out timer))
            {
                timer.Stop();
                timer.IsEnabled = false;
                timers.Remove(namedInvocation);
            }
            timer = new DispatcherTimer(priority,dispatcher)
            {
                Interval = delay
            };
            timer.Tick += (s, e)
                =>
                {
                    action();
                };
            timers.Add(namedInvocation, timer);
            timer.IsEnabled = true;

        }
    }
}

Answer 1

I'm not an expert on multithreading at all, but I've attempted refactoring your code a bit. First of all, to code defensively, I suggest adding the readonly modifier to your lock object. It's not required but will prevent accidental assignments:

private static readonly object syncRoot = new object();

I'm against incurring the complexity of an out argument: let's just check if the timer exists. We can stop it and remove it if it does; setting DispatcherTimer.IsEnabled isn't necessary because DispatcherTimer.Stop() sets it to false anyway:

if (timers.ContainsKey(namedInvocation))
{
    timers[namedInvocation].Stop();
    timers.Remove(namedInvocation);
}

We can get rid of the object initializer because DispatcherTimer has the perfect constructor for us anyway; this has the added benefit of inlining the lambda:

var timer = new DispatcherTimer(delay, priority, (s, e) => action(), dispatcher);
timer.Start();
timers.Add(namedInvocation, timer);

Again, remember we don't need to set IsEnabled because DispatcherTimer.Start() sets it to true for us (alternatively, you can leave it in place but remove the call to Start(). But the method call seems more intuitive to me; setting a property doesn't make it obvious that the timer has been started).

You may have noticed I used var wherever it seemed appropriate; it's just as type safe as using the actual type but takes up less space, both on the page in in my brain.

That leaves us with the following end result:

public static class DispatcherExtensions
{
    private static Dictionary<string, DispatcherTimer> timers = 
        new Dictionary<string, DispatcherTimer>();
    private static readonly object syncRoot = new object();

    public static void DelayInvoke(this Dispatcher dispatcher, string namedInvocation,
        Action action, TimeSpan delay, 
        DispatcherPriority priority = DispatcherPriority.Normal)
    {
        lock (syncRoot)
        {
            if (timers.ContainsKey(namedInvocation))
            {
                timers[namedInvocation].Stop();
                timers.Remove(namedInvocation);
            }
            var timer = new DispatcherTimer(delay, priority, (s,e) => action(), dispatcher);
            timer.Start();
            timers.Add(namedInvocation, timer);
        }
    }
}

In case I introduced a bug or if you have improvements, let me know in the comments.

---

Edit: supporting cancellation

(@svick's suggestion)

To provide the possibility of cancelling your timers, you would factor out the timer removing logic (I inverted the condition to reduce nesting...) into a new method that is called at the beginning of DelayInvoke.

private static void RemoveTimer(string namedInvocation)
{
    if (!timers.ContainsKey(namedInvocation)) return;
    timers[namedInvocation].Stop();
    timers.Remove(namedInvocation);
}

Then, you'd create another extension method (first parameter is useless, but calling the method via the Dispatcher is consistent and convenient, so I left it there). Something like this:

public static void CancelNamedInvocation(this Dispatcher dispatcher, string namedInvocation)
{
    lock(syncRoot)
    {
        RemoveTimer(namedInvocation);
    }
}