Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

JavaScript implementation of a basic calculator. Looking for constructive criticism of my code. Thanks in advance!

    window.onload = function() {

    // number button variable declarations

    var zeroButton = document.getElementById("zero");
    var oneButton = document.getElementById("one");
    var twoButton = document.getElementById("two");
    var threeButton = document.getElementById("three");
    var fourButton = document.getElementById("four");
    var fiveButton = document.getElementById("five");
    var sixButton = document.getElementById("six");
    var sevenButton = document.getElementById("seven");
    var eightButton = document.getElementById("eight");
    var nineButton = document.getElementById("nine");

    // operator button variable declarations

    var plusButton = document.getElementById("plus");
    var minusButton = document.getElementById("minus");
    var timesButton = document.getElementById("times");
    var dividedByButton = document.getElementById("divided_by");

    // action button variable declarations

    var clearButton = document.getElementById("clear");
    var enterButton = document.getElementById("enter");

    // display variable declarations

    var operatorDisplay = document.getElementById("operator_display");
    var resultDisplay = document.getElementById("result_display_value");

    // variable holds hidden number

    var hiddenNum = "";

    // number button function

    var numberFunction = function(numberButton) {
        numberButton.onclick = function(event) {
            resultDisplay.innerHTML += numberButton.innerHTML;
        }
    };

    // number function calls

    numberFunction(zeroButton);
    numberFunction(oneButton);
    numberFunction(twoButton);
    numberFunction(threeButton);
    numberFunction(fourButton);
    numberFunction(fiveButton);
    numberFunction(sixButton);
    numberFunction(sevenButton);
    numberFunction(eightButton);
    numberFunction(nineButton);

    // operations function

    var operations = function() {
        if (operatorDisplay.innerHTML === "+") {
            resultDisplay.innerHTML = parseInt(hiddenNum) + parseInt(resultDisplay.innerHTML);
        } else if (operatorDisplay.innerHTML === "-") {
            resultDisplay.innerHTML = parseInt(hiddenNum) - parseInt(resultDisplay.innerHTML);
        } else if (operatorDisplay.innerHTML === "*") {
            resultDisplay.innerHTML = parseInt(hiddenNum) * parseInt(resultDisplay.innerHTML);
        } else if (operatorDisplay.innerHTML === "/") {
            resultDisplay.innerHTML = parseInt(hiddenNum) / parseInt(resultDisplay.innerHTML);
        }
    };

    // operator button function

    var operatorFunction = function(operatorButton) {
        operatorButton.onclick = function(event) {
            if (resultDisplay.innerHTML !== "" && hiddenNum !== undefined) {
                operations();
            }
            hiddenNum = resultDisplay.innerHTML;
            resultDisplay.innerHTML = "";
            operatorDisplay.innerHTML = operatorButton.innerHTML;
        }
    };

    // operator function calls

    operatorFunction(plusButton);
    operatorFunction(minusButton);
    operatorFunction(timesButton);
    operatorFunction(dividedByButton);

    // clear function

    var clearFunction = function(button, display) {
        display.innerHTML = "";
    };

    // clear button

    clearButton.onclick = function(event) {
        clearFunction(clearButton, operatorDisplay);
        clearFunction(clearButton, resultDisplay);
        hiddenNum = "";
    }

    // enter button

    enterButton.onclick = function(event) {
        if (resultDisplay.innerHTML === "" && hiddenNum === "") {
            resultDisplay.innerHTML = "";
        } else if (resultDisplay.innerHTML === "") {
            if (operatorDisplay.innerHTML === "+") {
                resultDisplay.innerHTML = parseInt(hiddenNum) + parseInt(hiddenNum);
            } else if (operatorDisplay.innerHTML === "-") {
                resultDisplay.innerHTML = parseInt(hiddenNum) - parseInt(hiddenNum);
            } else if (operatorDisplay.innerHTML === "*") {
                resultDisplay.innerHTML = parseInt(hiddenNum) * parseInt(hiddenNum);
            } else if (operatorDisplay.innerHTML === "/") {
                resultDisplay.innerHTML = parseInt(hiddenNum) / parseInt(hiddenNum);
            }
        } else {
            operations();
        }
        clearFunction(enterButton, operatorDisplay);
    }

}
share|improve this question

I think that using DOM elements as a data storage is a bad design choise. Use JS variables for that and the number of ugly .innerHTMLs in your code will decrease drammatically (in addition it is a long operation).

For example:

var operations = function() {
    if (operator === "+") {
        result = parseInt(hiddenNum) + parseInt(result);
    } else if (operator === "-") {
        result = parseInt(hiddenNum) - parseInt(result);
    } else if (operator === "*") {
        result = parseInt(hiddenNum) * parseInt(result);
    } else if (operator === "/") {
        result = parseInt(hiddenNum) / parseInt(result);
    }
    resultDisplay.innerHTML = result;
};

where result and operator is global variables.

Second, you can store number buttons in an array and name them like "button0", "button1" and so on.

var numButton = new Array(10)
for (i=0; i<10; i++) {
    numButton[i] = document.getElementById("button" + i);
    numButton[i].onclick = function(event) {
        // be careful with a scope of variable i here,
        // I dont sure it is a right variant
        result += i
        resultDisplay.innerHTML = result ;
    }
}
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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