Timer Sequence is Increasing On each go

Question

So I am making a Robbery system the timer works perfectly on the first start the sequence by the timer goes down is 10,9,8,7...... But on the 2nd try the sequence is 10,8,6,4...... on the 3rd try its 10,7,4,1..... Etc meaning on each start the timer sequence increase the decrease time? How is it possible? Can you make an edit on the code for me?

public int time;
Timer cd = new Timer(2000);
public void robberyCountdown(User player)
{
    time = 10;
    cd.Elapsed += (source, e) => OnTimer(player);
    cd.Start();
}

public void OnTimer(User player)
{
    cd.Stop();
    cd.Elapsed -= (source, e) => OnTimer(player);
}

but when i use (cd1.Elapsed += (Object source, ElapsedEventArgs e, User player) on OnTimer it gives me an error on the cd.Elapsed -= (source, e) => OnTimer(source, e, player); Line stating local variable is source cannot be used here as it is defined in this scope

it's just a define like this public int time; Timer cd = new Timer(2000); — CrazyGamer, Mar 10 '16 at 19:22
There are 3 main types of timers, is it a System.Windows.Forms.Timer? System.Timers.Timer? System.Threading.Timer? If you're not sure, you can hover over "Timer" in visual studio — spectacularbob, Mar 10 '16 at 19:32

score 2 · Answer 1 · edited May 23 '17 at 12:31

I suspect this is because your elapsed event handler isn't actually being removed, so it is firing multiple times at each tick.

The following question/answer discusses how to remove a delegate, and how Delegate.Equals determines which delegate gets removed (if any): C# removing an event handler

However, you are not removing the same handler, you are creating two separate lambda functions that perform exactly the same task within different contexts. .NET isn't very good at collapsing duplicate code - it cannot resolve that your second lambda is the same as your first because they are each created in a different scope, and therefore each has a different closure (the saved image of all visible variables within the scope where it is created).

If you create an actual non-lambda method, and then use delegates rather than lambdas, this problem should go away. Or, you may be able to assign the lambda to a variable that is accessible to both functions (without modifying it in between).

There is an example in Microsoft's documentation here: https://msdn.microsoft.com/en-us/library/system.timers.timer.elapsed(v=vs.110).aspx — Matt Jordan, Mar 10 '16 at 19:50

score 0 · Answer 2 · edited May 23 '17 at 10:28

This is a example implementation of Matt Jordan's solution "assign the lambda to a variable that is accessible to both functions"

public int time;
Timer cd = new Timer(2000);
public void robberyCountdown(User player)
{
    time = 10;
    ElapsedEventHandler handler = null;
    handler = (source, e) => OnTimer(player, handler);
    cd.Elapsed += handler;
    cd.Start();
}

public void OnTimer(User player, ElapsedEventHandler handler)
{
    cd.Stop();
    cd.Elapsed -= handler;
}

Because lambadas capture variables, not values, handler can reference itself so long as it is already been initialized to a value (null in this example) before you try to use it.

Timer Sequence is Increasing On each go

2 Answers2