Skip to content
This repository has been archived by the owner on Aug 24, 2019. It is now read-only.

Possible bug? #9

Closed
dmrev opened this issue Sep 14, 2015 · 9 comments
Closed

Possible bug? #9

dmrev opened this issue Sep 14, 2015 · 9 comments

Comments

@dmrev
Copy link

dmrev commented Sep 14, 2015

Line 67 in RateLimit.swift:

// Record execution
dictionary[name] = NSDate()

You record new execution time even if decide to not execute? Is that right?

@soffes
Copy link
Owner

soffes commented Sep 14, 2015

Yikes! You're right. I'll get this fixed ASAP.

@soffes
Copy link
Owner

soffes commented Sep 14, 2015

Fixed in 2a319ad. Version 1.1.1 is released with this fix.

@soffes soffes closed this as completed Sep 14, 2015
@soffes
Copy link
Owner

soffes commented Sep 16, 2015

Actually, it worked as expected before. Oops.

@dmrev
Copy link
Author

dmrev commented Sep 16, 2015

You wrong. Think again.
In your original implementation, if you call some block fast enough (time interval between calls < limit), than that block will never be executed after first call.
But if you record execution time only after successful call, block will be executed 1 time per limit. This is rate limit.

@soffes
Copy link
Owner

soffes commented Sep 16, 2015

With your proposed changed, if you hit it ever half second and set the limit to 1 second, it will only get called once.

@dmrev
Copy link
Author

dmrev commented Sep 16, 2015

Once per second.

@soffes
Copy link
Owner

soffes commented Sep 16, 2015

Try the demo app in version 1.2.0. It works as expected, no?

@dmrev
Copy link
Author

dmrev commented Sep 16, 2015

As YOU expected, yes. As I expected – no.
I expect that when i tap very fast (5 times per second, for example) label text will change once per second. This behaviour i will call "rate limit". Rule: "perform call, but no more often than once at the designated time interval."
But what we have now is "call canceller". Because of rule "cancel call if it happens earlier than necessary".

@dmrev
Copy link
Author

dmrev commented Sep 16, 2015

However, this is not bug, this is design decision. If you want such behaviour, it's ok.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants