OpenTechSchool / js-beginners-1

Curriculum for the first part of JavaScript workshop for absolute beginners.
http://opentechschool.github.io/js-beginners-1/
27 stars 21 forks source link

Better implementation of setInterval #23

Open gabeno opened 10 years ago

gabeno commented 10 years ago

The introduction to the function setInterval says: setInterval(func, n) is a way to run a function every n milliseconds yet during implementation n has to be computed like so:

// Draw twenty frames per second (20 * 50 = 1000 milliseconds)
setInterval(frame, 50);

This is confusing. For consistency we could have the function take in millisecond units or have an explanation for the conversion.

theophani commented 10 years ago

Hey :). setInterval is a native method to JavaScript and it takes milliseconds as the second argument. The comment is attempting to explain that if you want to draw a frame 20 times in one second, then you need to refresh every 50 milliseconds.

Perhaps this explanatory comment would be more clear if it said:

// Draw twenty frames per second (1000 / 20 = 50 milliseconds per frame)
setInterval(frame, 50);
gabeno commented 10 years ago

Hi @theophani, your explanation is better, thank you. The main idea is to have x number of frames drawn per second. Once x is chosen the time per frame is then calculated. :)

xMartin commented 10 years ago

So should we improve the material?

gabeno commented 10 years ago

@xMartin I think we should improve the comments to be more concise and easy to understand for a beginner programmer. I propose this:

\\ Set number of frames to be drawn per second at 20 (n = 1000 / 20 = 50 milliseconds)

The simple arithmetic better explains how the n in setInterval(func, n) is arrived at.

xMartin commented 7 years ago

Should still be improved, I guess.

tobmaster commented 7 years ago

I was thinking of expressing whats going on in source code with good named variables. Besides making it less abstract, it also teaches good practices to make the code "speaking".

// We want to draw twenty frames per second 
var framesPerSecond = 20;

// 1 second equals 1000 milliseconds.
var frameInterval = 1000 / framesPerSecond;

// setInterval expects the interval in milliseconds
setInterval(frame, frameInterval);
xMartin commented 7 years ago

Personally, I like your version a lot. I've experienced learners that found it much easier to understand code with lots of names, that were overwhelmed with magic numbers.

On the other hand, some people do think in a more mathematical way and like the condensed way, playing with the numbers in place.

I wonder if we can provide multiple versions of code. But that's probably going too far.

With that said, I still think your version would be an improvement for most learners ;) Maybe we can add to the comments that this is equal to "draw a frame every 50 milliseconds".

tobmaster commented 7 years ago

Yeah but "mathematical thinkers" will be able to handle the variable names too.

xMartin commented 7 years ago

@tobmaster Could you prepare a Pull Request? Would be highly appreciated.

tobmaster commented 6 years ago

@xMartin sure