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

JSLint says "unexpected function" doCalc

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
jslintfunctiondocalcunexpectedsays

Problem

See the full calculator with HTML -> https://gist.github.com/1861120
You should be able to drag that html file from github into a browser and start using it. I'm only putting the js below. The calculator works fine but JS Lint is telling me I have an unexpected 'function' doCalc line 9 character 1.

Also just looking for some general tips on best practices and how this might be refactored. Any help is appreciated. Thank you.

var type, n, r, formula;
Math.factorial = function (n) {
    if (n < 2) {
        return 1;
    }
    return (n * Math.factorial(n - 1));
}

function doCalc(type) {
    switch (type) {
        case 'permutation':
            n = document.permutation.T1.value;
            r = document.permutation.T2.value;
            formula = function() {
                document.permutation.T3.value = Math.factorial(n) / Math.factorial(n - r);
            }
            break;
        case 'combination':
            n = document.combination.T1.value;
            r = document.combination.T2.value;
            formula = function() {
                document.combination.T3.value = Math.factorial(n) / (Math.factorial(r) * Math.factorial(n - r));
            }
            break;
    }
    n = Number(n);
    r = Number(r);
    if (isNaN(n) || isNaN(r)) {
        alert("Factorial requires a numberic argument.");
        return null;
    }
    formula();
}

window.onload = function() {
    document.getElementById('calculate-p').onclick = function() {
        doCalc('permutation');
    }
    document.getElementById('calculate-c').onclick = function() {
        doCalc('combination');
    }
}

Solution

Well, you asked for general tips, so here goes ;)

I would almost always refrain from extending built-in types. I will "shim" to provide standard-compliant functions which are not available (e.g. "forEach" on Array.prototype), but adding new methods to existing types is a recipe for problems. You could simply have a factorial function without the Math and it would work just as well.

In fact, you could do yourself a favor and create your own Math object (named something different, of course) and add your formula functions to that, so:

var MathFactor = new function () {
    this.factorial = function (n) {
        return n < 2 ? 1 : n * this.factorial(n - 1);
    };
    this.permutate = function (n, r) {
        return this.factorial(n) / this.factorial(n - r);
    };
    this.combine = function (n, r) {
        return this.factorial(n) / (this.factorial(r) * this.factorial(n - r));
    };
};


Then doCalc is just a matter of a simple numeric check, then setting your T3.value to the output of a call to MathFactor.permutate() or MathFactor.combine(). Hope that helps!

Code Snippets

var MathFactor = new function () {
    this.factorial = function (n) {
        return n < 2 ? 1 : n * this.factorial(n - 1);
    };
    this.permutate = function (n, r) {
        return this.factorial(n) / this.factorial(n - r);
    };
    this.combine = function (n, r) {
        return this.factorial(n) / (this.factorial(r) * this.factorial(n - r));
    };
};

Context

StackExchange Code Review Q#12076, answer score: 3

Revisions (0)

No revisions yet.