Naming
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 keyword
Better 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 cached
var $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('<div class="drop" id="drop' + i + '" style="left: ' + dropLeft + 'px; top: ' + dropTop +'px;"></div>');
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 < numberOfDrops; i++) {
var dropLeft = randRange(0, 1600);
var dropTop = randRange(-1000, 1400);
rainDrops += '<div class="drop" style="left: ' + dropLeft + 'px; top: ' + dropTop + 'px;"></div>';
}
$('.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 element
document.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 windowHeight = window.innerHeight;
var rainDrops = '';
for (var i = 0; i < numberOfDrops; i++) {
var dropLeft = getRandomNumberInRange(0, windowWidth);
var dropTop = getRandomNumberInRange(-1000, windowHeight);
rainDrops += '<div class="drop" style="left: ' + dropLeft + 'px; top: ' + dropTop + 'px;"></div>';
}
document.querySelector('.rain').innerHTML = rainDrops;
}
createRain(250);
}());
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;
}
}
<section class="rain"></section>