Requset::delay() waits too long

[GoogleCodeExporter]

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

[GoogleCodeExporter]

Correction

"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."

lastCallTime is not set to null, it is set to Now. Which means that the 
difference will always be less than 1 second.

Original comment by xXBiohaz...@gmail.com on 4 Jan 2012 at 1:28