HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Wrapping JS file in function, and assigning that function to a variable

Submitted by: @import:stackexchange-codereview··
0
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.

// 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 '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.