Today I worked on a project and found a piece of code, that left me wondering what the one who wrote it was actually thinking this should do. I know what the code is intended to do, but it was so weird, I wanted to share and try to find a better solution.
First of all the code, I didn’t copy & paste it, I wrote it from memory, my brain memory 🙂
Don’t copy and paste this
CancellationTokenSource _cancellationTokenSource = null;
private async void SetText()
{
try
{
if (_cancellationTokenSource != null && !_cancellationTokenSource.IsCancellationRequested)
{
_cancellationTokenSource.Cancel();
}
Interlocked.Exchange(ref _cancellationTokenSource, new CancellationTokenSource());
await Task.Delay(2000, _cancellationTokenSource.Token);
DoSearch();
}
catch
{
}
finally
{
if (!_cancellationTokenSource.IsCancellationRequested)
_cancellationTokenSource.Cancel();
_cancellationTokenSource.Dispose();
}
}
Any idea, what this is all about?
So let me just lift the curtain here. The function is being called within the Property-Setter, which is bound to an Entry. DoSearch() is just taking the Text and performing a full text search through a list of data. The challenge here is, that we don’t want to trigger the search on each keystroke, but when the user pauses for a second (or two in this case). Technically this code is producing a CancellationToken on each keystroke, waits 2 seconds, and cancels the previous delay task. TaskCanceledExceptions will be tolerated and ignored.
Try to make it better
If you have no clue, why this is bad code, you should just leave now. Really, go away. Let’s try to find a better solution. How about a counter? Let’s think of it like house, and every keystroke is a person going into that house. It’s a very special house, people are only visiting the house 2 seconds, then they leave. It’s not a brothel, maybe it stinks in the house, so the people leave right away. We stand at the port and count everyone that’s going and subtracting the peopel going out. After we reach to zero, we just close the door and go home, or whatever action we intent.
public class DelayedAction
{
private Action _action;
private int _callCounter = 0;
private TimeSpan _delay = TimeSpan.FromSeconds(1);
public DelayedAction(Action action)
{
_action = action;
}
public DelayedAction SetDelay(TimeSpan delay)
{
_delay = delay;
return this;
}
public async void Trigger()
{
Interlocked.Increment(ref _callCounter);
// now wait
await Task.Delay(_delay);
Interlocked.Decrement(ref _callCounter);
if (_callCounter == 0)
{
_action();
}
}
}
That’s it! Just a counter. No Exceptions, no CancellationTokens. We still have the Task.Delay but that’s way better than doing a SpinWait or BusyWait.
And here is how you can use it:
string _text;
DelayedAction _callSearchAction;
public MainPageViewModel(INavigationService navigationService)
: base(navigationService)
{
_callSearchAction = new DelayedAction(DoSearch)
.SetDelay(TimeSpan.FromMilliseconds(2000));
}
public string Text {
get => _text;
set
{
SetProperty(ref _text, value);
//SetText();
_callSearchAction.Trigger();
}
}