0
\$\begingroup\$

I decided to write a class to do this for me. I had a lot of methods that needed to be invoked at sporadic intervals, across multiple projects.

The dictionary will eventually be class methods, I just put new Task()... as an example. I'm fairly new to asynchronous programming, so constructive criticism is 100% welcome.

I set a low interval (10s) on the main timer because I want to stay as close to the interval in the dictionary as possible. I'm not sure the lowest I can put it without ruining performance.

At some point, I plan to implement non async methods. If anyone has any ideas on how I should implement that, please let me know. I feel like running a non async method inside a task would be a bad idea, maybe a seperate dictionary and process method would work?

Thanks.

public class AppTaskWorker : IAppTaskWorker
{
    private readonly ILogger _logger;
    private readonly ConcurrentDictionary<Task, DateTime> _tasksLog;
    private readonly ConcurrentDictionary<int, Task> _tasks;

    public AppTaskWorker()
    {
        _logger = new ConsoleLogger(typeof(AppTaskWorker));
        _tasksLog = new ConcurrentDictionary<Task, DateTime>();
        _tasks = new ConcurrentDictionary<int, Task>();

        TryRegisterTask(30, new Task(() => // every 30 seconds
        {
            Console.WriteLine("Task 1 has been processed.");
        }));

        TryRegisterTask(120, new Task(() => // every 2 minutes
        {
            Console.WriteLine("Task 2 has been processed.");
        }));

        TryRegisterTask(25, new SomeController().Process()); // every 25 seconds

        Start();
    }

    public void Start()
    {
        while (true)
        {
            Task.Run(async () =>
            {
                await ProcessActionsAsync();
                await Task.Delay(10000);
            });
        }
    }

    public bool TryRegisterTask(int interval, Task task)
    {
        return _tasks.TryAdd(interval, task);
    }

    private async Task ProcessActionsAsync()
    {
        foreach (var keyValuePair in _tasks)
        {
            var task = keyValuePair.Value;

            if (!_tasksLog.TryGetValue(task, out var lastProcessed))
            {
                continue;
            }

            if (!((DateTime.Now - lastProcessed).TotalSeconds >= keyValuePair.Key))
            {
                continue;
            }

            _tasksLog[task] = DateTime.Now;

            try
            {
                await task;
            }
            catch (Exception e)
            {
                _logger.Error(e.ToString());
            }
        }
    }
}
\$\endgroup\$
2
\$\begingroup\$

Start() is spawning an infinite number of new tasks without waiting for them to complete. It's also a bit weird that Start() is blocking the calling thread.

Make Start() async and await the Task returned by Task.Run.

Start(), a blocking method, being called at the end of the constructor is an absolute no-go. The constructor is not done, the object initialization is never completed.


Unless you're living on UTC (DateTime.Now - lastProcessed).TotalSeconds is going to be inconsistent around daylight saving transitions.

\$\endgroup\$
  • \$\begingroup\$ I've appended .Wait to the task call to wait for it. I wouldn't use Start() inside a constructor at production, it gets called outside the constructor now. Thanks for your answer! \$\endgroup\$ – a5266680 Aug 15 '18 at 14:49

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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