patternjavascriptMinor
Wrapping JS file in function, and assigning that function to a variable
Viewed 0 times
filefunctionthatwrappingandvariableassigning
Problem
I've been reading about wrapping JS files in functions to call them on load, etc. I also was looking at namespaces and how to avoid collisions with functions/objects/variables of the same name. What improvements can be made? I know the code works, I'm more concerned with best practices for wrapping JS files and using namespaces.
Both of the "public" functions would be called via
Also,
// codereview.js:
var codeReview = (function() {
// basically "private" function? Will there be a collision with jQuery here?
function $(id) {
return document.getElementById(id);
}
// "public" functions
return {
fizzbuzz: function() {
var newhtml = '';
var output = 0;
for (var i = 1; i ";
}
$('fizzbuzz-output').innerHTML = newhtml;
},
email: function() {
var first = "test@em";
var last = "ail.com";
var email = first + last;
$('my_email').innerHTML = email;
}
}
})();Both of the "public" functions would be called via
codeReview.fizzbuzz() or codeReview.email()Also,
codeReview.email() is basically just to click and reveal my email address. I did it this way thinking it couldn't get scraped, is that accurate or did I just come up with a super complicated way to display an email address?Solution
From a organizing perspective you are doing fine, in essence you used the Module Pattern. I would just advise you to consider placing
As you probably figured out,
For the email thing, I think it might fool a number of scrapers, I'd keep it.
For fizzbuzz, while your code is working, I think the goal is to use a minimum amount of modulo statements, your code could be written with fewer modulo statements..
My counter suggestion is very close to your code:
This was an interesting read, which provided an alternative inner loop which is more code review friendly:
'use strict'; in the beginning of your function.As you probably figured out,
$ does not collide with jQuery. Though, for this piece of code I would not create a one line function if you are only going to call that function once.For the email thing, I think it might fool a number of scrapers, I'd keep it.
For fizzbuzz, while your code is working, I think the goal is to use a minimum amount of modulo statements, your code could be written with fewer modulo statements..
My counter suggestion is very close to your code:
// codereview.js:
var codeReview = (function() {
// "public" functions
return {
fizzbuzz: function() {
var output, i;
for (i = 1; i <= 100; i++) {
output = '';
if (i % 3 == 0) {
output = 'Fizz';
}
if (i % 5 == 0) {
output += 'Buzz';
}
output = output || i;
console.log(output);
}
},
email: function() {
var first = 'test@em';
var last = 'ail.com';
var email = first + last;
return document.getElementById('my_email').innerHTML = email;
}
}
})();
codeReview.fizzbuzz();This was an interesting read, which provided an alternative inner loop which is more code review friendly:
for (i = 1; i <= 100; i++) {
if (i % 15 == 0) {
output = 'FizzBuzz';
}
else if (i % 3 == 0) {
output = 'Fizz';
}
else if (i % 5 == 0) {
output += 'Buzz';
}
else{
output = i;
}
console.log(output);
}Code Snippets
// codereview.js:
var codeReview = (function() {
// "public" functions
return {
fizzbuzz: function() {
var output, i;
for (i = 1; i <= 100; i++) {
output = '';
if (i % 3 == 0) {
output = 'Fizz';
}
if (i % 5 == 0) {
output += 'Buzz';
}
output = output || i;
console.log(output);
}
},
email: function() {
var first = 'test@em';
var last = 'ail.com';
var email = first + last;
return document.getElementById('my_email').innerHTML = email;
}
}
})();
codeReview.fizzbuzz();for (i = 1; i <= 100; i++) {
if (i % 15 == 0) {
output = 'FizzBuzz';
}
else if (i % 3 == 0) {
output = 'Fizz';
}
else if (i % 5 == 0) {
output += 'Buzz';
}
else{
output = i;
}
console.log(output);
}Context
StackExchange Code Review Q#61938, answer score: 4
Revisions (0)
No revisions yet.