Writing C#'s analog of setTimeout, setInterval and clearInterval

Question

I got tired of starting timer every time I need to do something simple as running some function 5 seconds after something in my code happens. So I tried to write a function (actually, 3 of them: setTimeout, setInterval and clearInterval) to write one line of code when I need it instead of, say, five.

It is working, at least for the cases I tested.

I understand that maybe something in that code is not right, maybe I forgot about something. Should I improve anything in my approach?

using System;
using System.Windows.Forms;

namespace timeouts_test
{
    public partial class Form1 : Form
    {
        private Timer[] timerList = new Timer[100];
        private int timerMaxIndex = -1;
        public Form1()
        {
            InitializeComponent();
            int aInterval = SetInterval(500, a);
            int bInterval = SetInterval(500, b);
            int cInterval = SetInterval(500, c);

            SetTimeout(3500, () =>
            {
                ClearInterval(aInterval);
            });

            SetTimeout(7000, () =>
            {
                ClearInterval(bInterval);
            });

            SetTimeout(10500, () =>
            {
                ClearInterval(cInterval);
            });
        }

        public void a()
        {
            label1.Text += "a";
        }

        public void b()
        {
            label1.Text += "b";
        }

        public void c()
        {
            label1.Text += "c";
        }

        public int SetInterval(int time, Action function)
        {
            Timer tmr = new Timer();
            tmr.Interval = time;
            tmr.Tick += new EventHandler(delegate (object s, EventArgs ev)
            {
                function();
            });
            tmr.Start();

            timerMaxIndex++;
            var index = timerMaxIndex;
            timerList[timerMaxIndex] = tmr;            
            return index;
        }

        public int SetTimeout(int time, Action function)
        {
            Timer tmr = new Timer();
            tmr.Interval = time;
            tmr.Tick += new EventHandler(delegate (object s, EventArgs ev)
            {
                function();
                tmr.Stop();
            });
            tmr.Start();

            timerMaxIndex++;
            var index = timerMaxIndex;
            timerList[timerMaxIndex] = tmr;            
            return index;
        }

        public void ClearInterval(int interval)
        {
            if (timerList[interval] == null)
                return;

            timerList[interval].Stop();
            timerList[interval] = null;
            if (interval == timerMaxIndex)
                timerMaxIndex--;
        }
    }
}

(Functions a, b and c were made just to test this.)

Answer 1

The following is from Wikipedia

Modularity, and hence separation of concerns, is achieved by encapsulating information inside a section of code that has a well-defined interface

That's why we should first think about the primary concerns of our application, and try to seperate these concerns into classes, which can then also be re-used if designed modularity in mind.

Therefore, Assuming what you are trying to achieve is something like this:

ExecutionPlan aInterval = ExecutionPlan.Repeat(500, a);
ExecutionPlan bInterval = ExecutionPlan.Repeat(500, b);
ExecutionPlan cInterval = ExecutionPlan.Repeat(500, c);

ExecutionPlan.Delay(3500, () => { aInterval.Abort(); });
ExecutionPlan.Delay(7000, () => { bInterval.Abort(); });
ExecutionPlan.Delay(10500, () => { cInterval.Abort(); });

You can wrap your timers in ExecutionPlan class.

Using a class for wrapping the timers have many benefits.

1. You keep track of class instances and not bare integer values,
2. You can re-use this class in other projects or forms (if designed & implemented correctly),
3. You can extend this class to enrich it's functions
4. etc.. (There are an infinite number of benefits)

Here is my ExecutionPlan class which works with an underlying System.Timers.Timer:

public class ExecutionPlan : IDisposable
{
    private System.Timers.Timer planTimer;
    private Action planAction;
    bool isRepeatedPlan;

    private ExecutionPlan(int millisecondsDelay, Action planAction, bool isRepeatedPlan)
    {
        planTimer = new System.Timers.Timer(millisecondsDelay);
        planTimer.Elapsed += GenericTimerCallback;
        planTimer.Enabled = true;

        this.planAction = planAction;
        this.isRepeatedPlan = isRepeatedPlan;
    }

    public static ExecutionPlan Delay(int millisecondsDelay, Action planAction)
    {
        return new ExecutionPlan(millisecondsDelay, planAction, false);
    }

    public static ExecutionPlan Repeat(int millisecondsInterval, Action planAction)
    {
        return new ExecutionPlan(millisecondsInterval, planAction, true);
    }

    private void GenericTimerCallback(object sender, System.Timers.ElapsedEventArgs e)
    {
        planAction();
        if (!isRepeatedPlan)
        {
            Abort();
        }
    }

    public void Abort()
    {
        planTimer.Enabled = false;
        planTimer.Elapsed -= GenericTimerCallback;
    }

    public void Dispose()
    {
        if (planTimer != null)
        {
            Abort();
            planTimer.Dispose();
            planTimer = null;
        }
        else
        {
            throw new ObjectDisposedException(typeof(ExecutionPlan).Name);
        }
    }
}

Answer 2

Right now your timer methods have dependencies on stuff that shouldn't be.

You're relying on System.Windows.Form. That's blocking you if you ever want to use these methods in a console application or well... stuff that isn't related to WinForms. Check for the System.Timers.Timer class!

Your helper methods are relying on variables that aren't in their scope (The Timer[]). That's a dependency you want to remove.

To do so, we need a way to stop the timer. We could return the timer, but that might be a little too much for what we want to do. So let's create an interface, IInterruptable.

public interface IInterruptable
{
    void Stop();    
}

Now, we need something to wrap our Timer so we can implement the interface.

public class TimerInterrupter : IInterruptable
{
    private readonly Timer _timer;

    public TimerInterrupter(Timer timer)
    {
        if (timer == null) throw new ArgumentNullException(nameof(timer));
        _timer = timer;
    }

    public void Stop()
    {
        _timer.Stop();
    }
}

So far so good. That deals with the problem of the Timer[].

Next, let's see how we can change your implementation using System.Timers.Timer.

Point one, not totally related to the new implementation but very related to your old one. Using delegate is a little old-school. Since C#... 3? There are Functions you can use, that are backward compatible to delegate. That means :

//Syntax might not be good, but you get the point
fooBar.SomeEvent += new delegate(object,object){asd();};

Becomes :

fooBar.SomeEvent += (a,b) => asd();

So, let's look at that timer! This Timer class as an AutoReset property, which well... auto-resets the timer at the end of each interval is set to true. Which will be the main difference between your SetInterval and SetTimeout methods.

public static IInterruptable SetInterval(int interval, Action function)
{
    Action functionCopy = (Action)function.Clone();
    Timer timer = new Timer { Interval = interval, AutoReset = true };
    timer.Elapsed += (sender, e) => functionCopy();
    timer.Start();

    return new TimerInterrupter(timer);
}

Notice that the method is now static, since it's an helper method. You'll be able to use it in any context, which is great!

I changed the time parameter to interval, as it's clearer this way. You didn't pass a time, you passed a time interval as a parameter. It's important to be very clear.

I'm copying the Action. I'm not 100% sure that's useful, but it's to avoid a problem with references. If for example I did :

Action someFunction = CreateAFunctionThatIsntNull();
SetInterval(1,someFunction);
someFunction = null;

You're screwed. So copying the Action has a purpose here.

Now the SetTimeout :

public static IInterruptable SetTimeout(int interval, Action function)
{
    Action functionCopy = (Action)function.Clone();
    Timer timer = new Timer { Interval = interval, AutoReset = false };
    timer.Elapsed += (sender, e) => functionCopy();
    timer.Start();

    return new TimerInterrupter(timer);
}

Notice something? Well, that's the same code, with one different variable in AutoReset. So let's take care of that by extracting another method :

public static class TimerHelper
{
    public static IInterruptable SetInterval(int interval, Action function)
    {
        return StartTimer(interval, function, true);
    }

    public static IInterruptable SetTimeout(int interval, Action function)
    {
        return StartTimer(interval, function, false);
    }

    private static IInterruptable StartTimer(int interval, Action function, bool autoReset)
    {
        Action functionCopy = (Action)function.Clone();
        Timer timer = new Timer { Interval = interval, AutoReset = autoReset };
        timer.Elapsed += (sender, e) => functionCopy();
        timer.Start();

        return new TimerInterrupter(timer);
    }
}

That's the final version in my opinion. I think it looks better. Though there's a final problem. SetTimeout and SetInterval are terrible method names. I know the idea is to copy the Javascript functions, but let's make an exception. These methods names are not intuitive and overall pretty terrible. Maybe something like ExecuteEvery instead of SetInterval and ExecuteIn instead of SetTimeout would be better names.