patternjavascriptMinor
JavaScript Rain
Viewed 0 times
javascriptrainstackoverflow
Problem
I am looking for someone to help me improve this code. It works great but can sometimes slow my browser down a lot when raindrops are at a high amount.
html {
height: 100%;
}
body {
background: #0D343A;
background: -webkit-gradient(linear, 0% 0%, 0% 100%, from(rgba(13, 52, 58, 1)), to(#000000));
background: -moz-linear-gradient(top, rgba(13, 52, 58, 1) 0%, rgba(0, 0, 0, 1) 100%);
overflow: hidden;
}
.drop {
background: -webkit-gradient(linear, 0% 0%, 0% 100%, from(rgba(13, 52, 58, 1)), to(rgba(255, 255, 255, 0.6)));
background: -moz-linear-gradient(top, rgba(13, 52, 58, 1) 0%, rgba(255, 255, 255, .6) 100%);
width: 1px;
height: 89px;
position: absolute;
bottom: 200px;
-webkit-animation: fall .63s linear infinite;
-moz-animation: fall .63s linear infinite;
}
/* animate the drops*/
@-webkit-keyframes fall {
to {
margin-top: 900px;
}
}
@-moz-keyframes fall {
to {
margin-top: 900px;
}
}
// number of drops created.
var nbDrop = 1000;
// function to generate a random number range.
function randRange(minNum, maxNum) {
return (Math.floor(Math.random() * (maxNum - minNum + 1)) + minNum);
}
// function to generate drops
function createRain() {
for (i = 1; i ');
$('#drop' + i).css('left', dropLeft);
$('#drop' + i).css('top', dropTop);
}
}
// Make it rain
createRain();
Solution
Naming
If you need comment to explain a variable then the variable is not named correctly.
Variable
Instead of
Unnecessary Comments
Now, after changing the variables/functions name as shown in above section comment associated with them can be removed.
Below comments are unnecessary:
Or, in other words, you may rename the function to
Globals
Implicit Global
Variable
Better use of jQuery
Using method chaining, above code can be written as
Or better,
It works great but can sometimes slow my browser down a lot when raindrops are at a high amount.
Main reason behind this could be more number of DOM dives.
Each time
The question is Can we reduced these dives? And how much?
Yes. We can reduce the dives by caching the elements.
Here, we've cached the element which can be referenced inside the loop. This will bring the dives to 2001(one for
After chaining/using
Can we reduce this further?
Yes. Instead of applying styles after element is added to DOM, we can add styles to element and then throw it in DOM.
Did this actually reduced the DOM dives?
Let's use old formula here:
Using this, DOM dives will reduced to one. peace!.
Who needs jQuery?
Looking at above code, I realize jQuery is used only once to append the drops in container. append? The function is called only once, so we're not even actually appending(adding to existing content) them, we're setting(by setting I mean to add them and not append) them.
So, we can remove dependency on jQuery completely and use
It's funny to put this right after above section
Magic Numbers
Instead of using them directly, create a variable(of course with a descriptive name) and then use that variable.
We can infer them as screen width and screen height respectively. But how did you know my screen resolution? Or even, is that my screen resolution?
JavaScript provides a better way to get the width and height of browser. So,
Configurable number of drops
By passing the
Enough of talking, show me the code now!
`(function() {
'use strict';
function getRandomNumberInRange(minimum, maximum) {
return Math.floor(Math.random() * (maximum - minimum + 1)) + minimum;
}
function createRain(numberOfDrops) {
numberOfDrops = numberOfDrops || 1000;
var windowWidth = window.innerWidth;
var w
If you need comment to explain a variable then the variable is not named correctly.
Variable
nbDrop and functions randRange and createRain are accompanied with comments explaining their purpose. This is indication of non-descriptive names.Instead of
nbDrop, numberOfDrops is self-explanatory name and does not require comment explaining what the variable contains. Similarly, randRange can be renamed to getRandomNumberInRange. Although, the name is looking long, it's descriptive.Unnecessary Comments
Now, after changing the variables/functions name as shown in above section comment associated with them can be removed.
Below comments are unnecessary:
// Make it rain
createRain();createRain is pretty expressive. Anyone reading code can infer by reading the function name that it must be creating rain. So, the comment is not required.Or, in other words, you may rename the function to
makeRain and then remove comment. Which one to choose depends on your taste.Globals
nbDrop, randRange, createRain are globals. While it doesn't affect when working on small projects, it'll interfere with other developers code when working with large projects in big team due to variable names collision. To solve this the code can be wrapped in IIFE. See What is the (function() { } )() construct in JavaScript?Implicit Global
Variable
i used in for is implicit global. It is not declared(no var, let, const) but used. See Declaring variables without var keywordBetter use of jQuery
$('#drop' + i).css('left', dropLeft);
$('#drop' + i).css('top', dropTop);Using method chaining, above code can be written as
$('#drop' + i).css('left', dropLeft)
.css('top', dropTop);Or better,
css() accepts object of styles(key-value pairs). Now, the code will become$('#drop' + i).css({
'left': dropLeft,
'top': dropTop
});It works great but can sometimes slow my browser down a lot when raindrops are at a high amount.
Main reason behind this could be more number of DOM dives.
Each time
$(selector) is used, jQuery has to dive into DOM, get the element and then perform operations on it. In the function createRain inside of for, three times DOM elements are referred($('.rain') and $('#drop' + i) twice). This means, 3000 dives for 1000 rain drops.The question is Can we reduced these dives? And how much?
Yes. We can reduce the dives by caching the elements.
$('.rain') can be moved before the for and cachedvar $rain = $('.rain');
for (i = 1; i < nbDrop; i++) {Here, we've cached the element which can be referenced inside the loop. This will bring the dives to 2001(one for
.rain, 2000 for '#drop' + i).After chaining/using
css(properties), number of dives will become 1001.Can we reduce this further?
Yes. Instead of applying styles after element is added to DOM, we can add styles to element and then throw it in DOM.
$('.rain').append('');Did this actually reduced the DOM dives?
append is also a dive. But, this code help us realize that ID attribute on each .drop is just used to apply styles on it. If we use this syntax, we can remove ID of element.Let's use old formula here:
Create a string.
loop:
append to string
end:
use string here.Using this, DOM dives will reduced to one. peace!.
var rainDrops = '';
for (var i = 0; i ';
}
$('.rain').append(rainDrops);Who needs jQuery?
Looking at above code, I realize jQuery is used only once to append the drops in container. append? The function is called only once, so we're not even actually appending(adding to existing content) them, we're setting(by setting I mean to add them and not append) them.
So, we can remove dependency on jQuery completely and use
innerHTML with document.querySelector to select elementdocument.querySelector('.rain').innerHTML = rainDrops;It's funny to put this right after above section
Magic Numbers
1600, 1400 what are they?Instead of using them directly, create a variable(of course with a descriptive name) and then use that variable.
We can infer them as screen width and screen height respectively. But how did you know my screen resolution? Or even, is that my screen resolution?
JavaScript provides a better way to get the width and height of browser. So,
1600 will become window.innerWidth and 1400 will become window.innerHeight.Configurable number of drops
By passing the
numberOfDrops as argument to createRain() will give more control on how many drops to be shown. If nothing is passed, default 1000 can be used.Enough of talking, show me the code now!
`(function() {
'use strict';
function getRandomNumberInRange(minimum, maximum) {
return Math.floor(Math.random() * (maximum - minimum + 1)) + minimum;
}
function createRain(numberOfDrops) {
numberOfDrops = numberOfDrops || 1000;
var windowWidth = window.innerWidth;
var w
Code Snippets
// Make it rain
createRain();$('#drop' + i).css('left', dropLeft);
$('#drop' + i).css('top', dropTop);$('#drop' + i).css('left', dropLeft)
.css('top', dropTop);$('#drop' + i).css({
'left': dropLeft,
'top': dropTop
});var $rain = $('.rain');
for (i = 1; i < nbDrop; i++) {Context
StackExchange Code Review Q#157525, answer score: 6
Revisions (0)
No revisions yet.