5
\$\begingroup\$

I'm rather new to C#, and found relevant subclassing examples surprisingly hard to come by. I've whipped up this class that appears to work, but am pretty sure this is not optimal:

private class ThrottledRestClient
{
    int pause;  // mininum ms between requests
    double rpm;  // max requests per minute
    long last_request_time;
    RestClient client;
    public ThrottledRestClient(string url, HttpBasicAuthenticator auth, double rpm = 60)
    {
        this.client = new RestClient(url);
        this.client.Authenticator = auth;
        this.rpm = rpm;
        this.pause = (int)((60 / rpm) * 1000);
    }

    public IRestResponse Execute(RestRequest request)
    {
        long now = DateTime.Now.Ticks;
        int wait = pause - (int)((now - this.last_request_time) / TimeSpan.TicksPerMillisecond);
        if (wait > 0)
        {
            //Console.WriteLine("Waiting ms: " + wait);
            Thread.Sleep(wait);
        }
        this.last_request_time = DateTime.Now.Ticks;

        return this.client.Execute(request);
    }
}

How could this be better? Is there an existing solution I haven't found?

\$\endgroup\$
  • \$\begingroup\$ Are these requests async or sync? \$\endgroup\$ – Dreamwalker Jan 23 '15 at 14:34
  • \$\begingroup\$ Sync, but I'm running batches of things that don't depend on each other. \$\endgroup\$ – Cees Timmerman Jan 23 '15 at 15:05
  • \$\begingroup\$ this is not thread safe. several thread will update and read last_request_time concurrently \$\endgroup\$ – skyde Mar 10 '17 at 22:59
4
\$\begingroup\$

I'm rather new to C#, and found relevant subclassing examples surprisingly hard to come by

You just needed to try a bit harder! You are doing pretty much what you were supposed to do logic wise. What you are trying to do here obviously (maybe), is a specialization of a RestClient, so the RestClient class is the best candidate to inherit from. From there you may override the Execute method.

Don't be afraid to use verbose variables. The rpm name could be firstly reminded as rotations per minute which doesn't really apply here. Also try to name your instances fields in camelCase, you may or may not prefix them with _.

I also didn't get why your pause is (60 * 1000) / rpm = 1000 with the default rpm, but one thing I know is that you don't need to store it in a field because it may be calculated. I would also recommend using Environment.TickCount instead of DateTime.Now.Ticks, which is more performant.

With those modifications the code would become the following:

public class ThrottledRestClient : RestClient
{
    private readonly int _requestsPerMinute;
    private int _lastRequestTime;

    public ThrottledRestClient(int requestsPerMinute)
    {
        _requestsPerMinute = requestsPerMinute;
    }
    public override IRestResponse Execute(IRestRequest request)
    {
        int elapsedTime = Environment.TickCount - _lastRequestTime;
        int pause = (60/_requestsPerMinute)*1000;
        int wait = pause - elapsedTime;
        if (wait > 0)
        {
            Thread.Sleep(wait);
        }
        var response = base.Execute(request);
        _lastRequestTime = Environment.TickCount;
        return response;
    }
}

EDIT: To answer you comment. You can use this client like you would use the previous one eg:

var client = new ThrottledRestClient(60);
client.Authenticator = new HttpBasicAuthenticator("user", "password");
//so on so forth;
client.BaseUrl = "http://google.pt";
client.Request(new RestRequest());
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ I used 60 / rpm in case the API specified something else, e.g. 1 request per minute, so the pause would then be 60000. I don't see why you mention a more performant alternative yet keep calculating the pause. \$\endgroup\$ – Cees Timmerman Jan 26 '15 at 8:40
  • \$\begingroup\$ @CeesTimmerman Gotcha. I was referening to Environment.TickCount. This Tickcount is more suitable to calculate an epalsed time then DateTime.Now.Ticks. \$\endgroup\$ – Bruno Costa Jan 26 '15 at 8:44
  • \$\begingroup\$ Indeed it is. Strange that C# uses two different concepts named Tick. \$\endgroup\$ – Cees Timmerman Jan 26 '15 at 8:47
  • \$\begingroup\$ How do I tell your code what the base URL is? \$\endgroup\$ – Cees Timmerman Jan 26 '15 at 10:13
  • \$\begingroup\$ @CeesTimmerman edited the answer to address your question. \$\endgroup\$ – Bruno Costa Jan 26 '15 at 10:41
2
\$\begingroup\$

You might want a Generic throttling class see : Throttling class

var throttler = new Throttler(_requestsPerMinute, TimeSpan.FromMinutes(1));

And in your request processing simply call throttler.ThrottledWait(1);

public override IRestResponse Execute(IRestRequest request)
{
    throttler.ThrottledWait(1);
    return base.Execute(request);
}
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Is this thread-safe? \$\endgroup\$ – Cees Timmerman Apr 11 '17 at 9:01
  • \$\begingroup\$ @Cees Timmerman yes the Throttler class is thread-safe \$\endgroup\$ – skyde Apr 20 '17 at 21:16

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.