Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue #10: Random delay #13

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Issue #10: Random delay #13

wants to merge 3 commits into from

Conversation

gorork
Copy link

@gorork gorork commented Oct 26, 2014

What do you think, if Delay would be an array?

If there is 1 number in the array, the delay will be equal to this number.
If there are two numbers in the array, the delay would get random number in between.

What do you think, if Delay would be an array?

If there is 1 number in the array, the delay will be equal to this number.
If there are two numbers in the array, the delay would get random number in between.
@jkintscher
Copy link
Member

It’d make simulating a more real change rate easier, agreed. Thanks for making this work!
We’d need the CoffeeScript to be updated as well though. Probably better if you implement it in CS and then just have it compile it down to JS so it won’t get lost.

if (this.running) {
if (this.options.delay.length == 1) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This requires us to always pass an array as the delay, even if it’s set to a fixed number. It’d be better if it accepts either a number or an array. In the latter case, it uses the first and second element as the lower and upper boundaries for the random delay. So the conditional here would work as a type check for number and array.

But again, please make these changes in the CoffeeScript and just check in the compiled JS version.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, I believe it'd be more straightforward this way.

@gorork
Copy link
Author

gorork commented Oct 27, 2014

@jkintscher glad it fits! Never tried CoffeeScript before. Re-writing this in CS would be a nice excuse to try it! :)

…implemented in CoffeeScript and compiled JS).
@gorork
Copy link
Author

gorork commented Nov 2, 2014

@jkintscher I've added verification if Delay property is Array or Integer and refactored the code a bit. Please check both CS and JS files.

@jkintscher jkintscher mentioned this pull request Sep 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants