nikolaposa / rate-limit

🚔 General purpose rate limiter implementation.
MIT License
271 stars 47 forks source link

Custom interval required, why was Rate constructor set to final protected? #19

Closed virtualize closed 4 years ago

virtualize commented 4 years ago

We need to specify custom intervals, like 500 requests per 3 hours, or 30 per day, which is not possible without extending your Rate class.

Why did you choose to make the constructor final protected? I can see no valid reason to do so.

https://github.com/nikolaposa/rate-limit/blob/415c00c65eed8cf9326444757c0f1ff5ff0e793d/src/Rate.php#L17

I could do a PR if you approve. Thanks.

nikolaposa commented 4 years ago

final was to prevent someone from changing constructor behavior in the extending class, which would cause named constructors (perSecond(), perMinute(), etc.) to break.

protected as a visibility was only for the sake of encouraging the use of named constructors. I didn't expect those custom rates scenarios to be honest. But I guess I could live with public constructor.

Create a PR, I'll consider it.

virtualize commented 4 years ago

Thanks for merging the PR, could you do a minor release, so we can update composer?

nikolaposa commented 4 years ago

I'm using branch alias in my composer.json: https://github.com/nikolaposa/rate-limit/blob/master/composer.json#L51, so you should be able to require 2.0.* even without me releasing new version.

I'll probably wait for some time before minor release, to give chance for some other potential fixes and improvements.