Basic Calculator

I'm in a beginner JavaScript class and I feel like I have a pretty good grasp on all the intro material. All of my homework assignments work but they seem longer than they have to be, especially my most recent one. I'm not asking for answers to my homework as it is already complete (you can see the code works below) but I was seeing if anyone can give me some ideas for making my code more efficient.

Please don't provide me with new code and say paste this in as I really would like to think about it and learn. Maybe I haven't learned enough yet, but this looks pretty ugly to me from what I've seen online and I also repeat code... The only thing I'm required to do is build this with an object and there can only be one answer displayed at a time. I can't use jQuery or any other libraries.

```
var obj = {
num1 : document.getElementById('num1'),
num2 : document.getElementById('num2'),
add : document.getElementById("add"),
sub : document.getElementById("sub"),
mult : document.getElementById("mult"),
div : document.getElementById("div"),
result : document.getElementById("result"),
init : function() {
document.getElementById("calculate").onclick = obj.calc;
},
calc : function() {
var num1 = parseFloat(obj.num1.value);
var num2 = parseFloat(obj.num2.value);
if(isNaN(num1) || isNaN(num2)) {
alert("You must enter a number for First Number and Second Number.");
}
else if (num2 === 0) {
alert("You cannot divide by zero");
}
else {
if (obj.add.checked === true) {
var result = num1 + num2;
}
else if (obj.sub.checked === true) {
var result = num1 - num2;
}
else if (obj.mult.checked === true) {
var result = num1 * num2;
}
else if (obj.div.checked === true) {
var result = num1 /

rather than query each radio to see if it is checked and then run the arithmetic based upon that, I would probably store the arithmetic as a closure in your obj, where the key is the value of the checked input and then run document.querySelector to find the checked input, something like this:

var obj = {
 num1 : document.getElementById('num1'),
 num2 : document.getElementById('num2'),
 add  : function(n1, n2) { return n1 + n2; },
 sub  : function(n1, n2) { return n1 - n2; },
 mult : function(n1, n2) { return n1 * n2; },
 div  : function(n1, n2) { return n1 / n2; },
 result : document.getElementById("result"),
 init : function() {
     document.getElementById("calculate").onclick = obj.calc;
 },
 calc : function() {
     var num1 = parseFloat(obj.num1.value),
         num2 = parseFloat(obj.num2.value),
         operation, result;

     if(isNaN(num1) || isNaN(num2)) {
        return alert("You must enter a number for First Number and Second Number.");
     }
     else if (num2 === 0) {
        return alert("You cannot divide by zero"); 
     }

    operation = document.querySelector('input[type="radio"]:checked').value;

    result = obj[operation](num1, num2);

    obj.result.innerHTML = '';           

    p = document.createElement("p");
    p.setAttribute("id", "para");
    p.appendChild(document.createTextNode("The answer is " + result));
    obj.result.appendChild(p);
 }
 }

obj.init();

This saves you from having a really long series of if / else checking for the checked input. I would also remove the logic of checking whether you have already created a P and added it to the result div, instead i'd just set the result divs innerHTML to '' each time, and add the P tag again, as the end result is the same.

StackExchange Code Review Q#46053, answer score: 7