1
\$\begingroup\$

I'm messing around with JavaScript and am trying out objects, functions & arrays. I'd like to know if someone has tips about making it better, shorter, or has a better solution for what I have thought about for now. What I've tried to do is just do simple math (+ and -) using arrays as input for numbers. It all works, but I'm just wondering if it could be even better and/or cleaner.

//Write the result
function ap(n) {
    $(".value").append(n);
}

// Make sure the input is a number
function isNumber(n) {
    return !isNaN(parseFloat(n)) && isFinite(n);
}

function doMath(options) {
    var defaults = {
        method: 'add'
    };
    var options = $.extend({}, defaults, options);

    // Add
    if(options.method == 'add') {
        var n = 0;
        for(var i=0; i<options.numbers.length; i++) {
            if(isNumber(options.numbers[i])) {
                n = n + options.numbers[i];
            }
            if(i===options.numbers.length-1) {
                ap(n);
            }
        }
    }

    // Subtract
    if(options.method == 'subtract' ) {

        for (var i=0; i<options.numbers.length; i++) {
            if(i===0) {
                n = options.numbers[0];
            } else {
                if(isNumber(options.numbers[i])) {
                    n = n - options.numbers[i];
                }
            }

            if(i===options.numbers.length-1) {
                ap(n);
            }
        }
    }
}

//Print the result
doMath({'numbers': [10,5,2,2], 'method':'subtract'});
\$\endgroup\$

3 Answers 3

3
\$\begingroup\$

You should avoid mixing calculations with input/output. It would be better if your doMath() would not call the output function ap() directly, but instead would just return the result of its calculations:

ap(doMath({'numbers': [10,5,2,2], 'method':'subtract'}));

Of course the ap itself is a lousy name for a function, but it's more of a helper in here I guess.

I see no reason why you would use and options object instead of passing in just two parameters, but even when you use an options object, it would be simpler to extract its values into local variables and work with these. One can also easily drop the use of $.extend in favor of simple || operator, which is the canonical way of implementing default values for parameters:

    var method = options.method || "add";
    var numbers = options.numbers;

Another step to simplify code is to filter out the invalid numbers before doing any calculations on them. Array.filter helps us here:

var validNumbers = numbers.filter(isNumber);

The actual calculation can be easily implemented with Array.reduce. First we can define functions add and subtract

var methods = {
    add: function(a, b) { return a + b; },
    subtract: function(a, b) { return a - b; }
};

And then using Array.reduce we can apply them for the whole array:

validNumbers.reduce(methods.add);
// or
validNumbers.reduce(methods.subtract);

Pulling it all together:

function doMath(options) {
    var method = options.method || "add";
    var numbers = options.numbers;

    var validNumbers = numbers.filter(isNumber);

    var methods = {
        add: function(a, b) { return a + b; },
        subtract: function(a, b) { return a - b; }
    };

    return validNumbers.reduce(methods[method]);
}
\$\endgroup\$
0
1
\$\begingroup\$
function doMath(options) {
    var defaults = {
        method: 'add'
    };
    var options = $.extend({}, defaults, options);

    var n = 0;

    for(var i=0; i<options.numbers.length; i++) {
        if(isNumber(options.numbers[i])) {
            if(options.method == 'add' || i == 0) {
                n += options.numbers[i];
            }
            else if(options.method == "subtract"){
                n -= options.numbers[i];
            }
        }
    }
    ap(n);
}

This code is making many assumptions, for example that options.numbers is defined, but for the lack of the exact definition of the problem you are trying to solve this looks at least shorter.

\$\endgroup\$
0
\$\begingroup\$

Another solution (e.g. if you cannot use the Array-Methods):

function doMath(options){
    var result;
    var operations={
        add: function(){
            var result=0;
            return function(o){
                return result+=o;
            }
        }(),
        substract: function(){
            var result=0;
            return function(o){
                return result-=o;
            }
        }()
    };

    var fn=operations[options.method]||operations.add;

    while(number=options.numbers.pop()){
        result=fn(number);
    }
    return result;
}

Here is the Fiddle.

Not quite as elegant as Array.reduce() but does its job. (Validation for numbers omitted)

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.