Requset::delay() waits too long #10
Description
The last.fm API specifies that you should not issue more than 5 requests per
second. lastfm-sharp has the following code
private void delay()
{
// If the last call was less than one second ago, it would delay execution for a second.
if (Request.lastCallTime == null)
Request.lastCallTime = new Nullable<DateTime>(DateTime.Now);
if (DateTime.Now.Subtract(Request.lastCallTime.Value) > new TimeSpan(0, 0, 1))
Thread.Sleep(1000);
}
There are four things wrong with this:
1) If this is our first request (Request.lastCallTime == null), then we set the
lastCallTime to null. This guarantees that when we see how much time has passed
since our first request, it will be less than 1000ms, thus causing us to wait
for our first request.
2) The when we compare the time elapsed to the new TimeSpan, we are checking to
see if the duration is greater than 1 second, and if it is we wait 1 second. We
should be checking to see if it is less than 1 second.
3) The last.fm API designates that we should not exceed 5 requests per second.
The library currently limits us to 1 request per second.
4) We always wait 1 full second if not enough time has elapsed. So, for
example, if 999ms had elapsed since the last request, we will wait a full 1
second before our next request. Meaning that we have waiting 1999ms to send
another request.
The code would be better written as
private void delay()
{
if (Request.lastCallTime != null)
{
// If the last call was less than two hundred miliseconds ago
TimeSpan timeSinceLastCall = DateTime.Now.Subtract(Request.lastCallTime.Value);
TimeSpan timeToWaitBetweenCalls = new TimeSpan(0, 0, 0, 0, 200);
if (timeSinceLastCall < timeToWaitBetweenCalls)
Thread.Sleep(timeToWaitBetweenCalls - timeSinceLastCall);
}
}
Original issue reported on code.google.com by xXBiohaz...@gmail.com on 4 Jan 2012 at 1:21