0

I am working with timers and have implemented threading as first step in solving my problem. I have to send some reference every 30 seconds and this reference is created everytime a button is clicked. So I have the following code:

public MyReference SomeReference {get;set;}
public string MyFullName {get;set;} //this is bound to WPF textbox

public bool Saved()
{
     SomeReference.Id = GeneratedId;
     SomeReference.FullName = MyFullName;

     return SavedtoDB(SomeReference);
}

public void OnButtonClick()
{
    if(Saved()) //timer should only start if it is already saved in the database
         var t = new Thread(() => ExecuteTimer(SomeReference));
}

public void ExecuteTimer(MyReference someReference)
{
    System.Timers.Timer sendTimer = new System.Timers.Timer(30000);
    sendTimer.Elapsed += (sender, e) => ExecuteElapsed(sender, e, someReference, sendTimer);
    sendTimer.Start();
}

public void ExecuteElapsed(object source, ElapsedEventArgs e, MyReference someReference, System.Timers.Timer sendtimer)
{
    sendtimer.Stop();
    Send(someReference);
}

The following is the MyReference class:

public class MyReference()
{
    public string Id {get;set;}
    public string FullName {get;set;}
}

Edit for problem emphasis:

The problem is that it only sends the latest values someReference and disregards the previous ones. How will I send each values after 30 seconds without replacing the them with the latest value of someReference?

ThEpRoGrAmMiNgNoOb
  • 1,256
  • 3
  • 23
  • 46
  • Why are you stopping the timer after it elapses? Doesn't this make the timer itself obsolete since every timer created only runs once until it is being stopped? – Robin B Nov 06 '19 at 08:41
  • 2
    System.Timers.Timer already runs on a different thread. The code posted here creates *multiple* single-fire timers, all of which remain active, causing leaks. What are you trying to do? What is the *real* problem you want to solve? Why not use eg `Task.Delay` to add a delay to the button action? If you want to reset the timer on each click, you can stop and start it again – Panagiotis Kanavos Nov 06 '19 at 08:42
  • @RobinB I am stopping it so that it will elapse only once. if it will not be stopped, it will execute the elapse again after 30 seconds. – ThEpRoGrAmMiNgNoOb Nov 06 '19 at 08:43
  • 4
    The only thing that's certain here is that the thread isn't needed. – Panagiotis Kanavos Nov 06 '19 at 08:43
  • @ThEpRoGrAmMiNgNoOb no it won't, the default behavior of System.Threading.Timer is to fire just once on a *separate thread*. You could create the timer just once in the constructor, store it in a field and just call `Start()` in the click handler if you wanted it to fire once – Panagiotis Kanavos Nov 06 '19 at 08:44
  • If the user clicks one time the button, what should happen? If the user clicks multiple times the button, what should happen then? – Theodor Zoulias Nov 06 '19 at 08:46
  • @TheodorZoulias if the user clicks the button one time, then it will wait 30 seconds to send the reference. If it clicks the button multiple times, then it should time each created reference 30 seconds before sending. – ThEpRoGrAmMiNgNoOb Nov 06 '19 at 08:49
  • So every time the user clicks the button, a reference should be send with a 30 sec delay, correct? – Theodor Zoulias Nov 06 '19 at 08:50
  • @TheodorZoulias yes – ThEpRoGrAmMiNgNoOb Nov 06 '19 at 08:51
  • It sounds like you need queue and single timer, but it's not clear what is expected behavior when button is spammed. – Sinatr Nov 06 '19 at 08:55
  • @Sinatr yes, if the button is spammed, it will queue all created `someReference` and will time each of them 30 seconds after creation and will send each afterwards. – ThEpRoGrAmMiNgNoOb Nov 06 '19 at 09:28
  • Shouldn't you create a `SomeReference` every time the button is clicked? How is this object created? – Theodor Zoulias Nov 06 '19 at 09:30
  • @TheodorZoulias I already added the code on how I create my SomeReference. – ThEpRoGrAmMiNgNoOb Nov 06 '19 at 09:37
  • @TheodorZoulias I made another edit to emphasize that it should only be sent if it's already saved in the DB. – ThEpRoGrAmMiNgNoOb Nov 06 '19 at 09:45
  • 1
    You mutate the object in `Saved`. Create a new one and all will work. `SomeReference = new MyReference();` as a first line in `Saved`. – lilo0 Nov 06 '19 at 09:48
  • Could you add a method [`Clone`](https://learn.microsoft.com/en-us/dotnet/api/system.icloneable.clone) to the `MyReference` class, so that you can create a clone of the `SomeReference` instance every time the button is clicked? This method should create a new `MyReference` instance, and copy all the important data from the original object to the clone, and then return the clone. – Theodor Zoulias Nov 06 '19 at 09:56

3 Answers3

2

It seems like you need a deep clone of your object.

Let me explain why a reference to your object is not enough. The SomeReference variable will always just point at a memory location where your MyReference instance is located. If you click your button this data is written/overwritten. When your timers fires it will always load whatever data is located at SomeReference and send it.

Implementing a deep copy will physically copy your instance in memory so you will keep a copy of the object which can be used to send without being overwritten from your Save() call.

To implement this you can (there are a lot of other methods) use the ICloneable interface like this:

  class MyReference : ICloneable
  {
    public string Id { get; set; }
    public string FullName { get; set; }

    public object Clone()
    {
      return new MyReference()
      {
        Id = this.Id,
        FullName = this.FullName
      };
    }
  }

Now when clicking the button you can use Theodors Task.Delay method and give it a copy of your object:

    public void OnButtonClick()
    {
      //Here we now copy the complete object not just the reference to it
      var localCopyOfTheReference = (MyReference)someReference.Clone();

      var fireAndForget = Task.Run(async () =>
      {
        await Task.Delay(30000);
        Send(localCopyOfTheReference);
      });
    }
Robin B
  • 1,066
  • 1
  • 14
  • 32
1

Here is how you can start a task that will send a reference with a 30 seconds delay, every time the button is clicked.

public void OnButtonClick()
{
    var localCopyOfTheReference = someReference;
    var fireAndForget = Task.Run(async () =>
    {
        await Task.Delay(30000);
        Send(localCopyOfTheReference);
    });
}

In case of an exception, the exception will be swallowed.

Theodor Zoulias
  • 34,835
  • 7
  • 69
  • 104
0

Maintaining all those timers seems really confusing.

Why not just maintain a list of times when you want the event to happen? And make the event a delegate.

ConcurrentDictionary<DateTime,Action> events = new ConcurrentDictionary<DateTime,Action>();

When someone clicks something,

events.TryAdd(DateTime.Now.AddSeconds(30), () => HandleEvent(whatever, you, want));

In the handler, make sure there isn't a later event.

void HandleEvent()
{
    if (events.Keys.Any( dateKey => dateKey > DateTime.Now )) return;

    DoStuff();
}

Now you just need a way to make the events happen.

timer.Elapsed += (s,e) => {
    var list = events.Where( item => item.Key <= DateTime.Now ).ToList();
    foreach (var item in list)
    {
        item.Value.Invoke();
        events.TryRemove(item);
    }
};
John Wu
  • 50,556
  • 8
  • 44
  • 80