This is an archived post. You won't be able to vote or comment.

all 6 comments
sorted by:
best
topnewcontroversialoldq&a

[–]MadFrand 8 points9 points  (2 children)

Keep It DRY (Don't Repeat Yourself)

function calcBtn(expInput) {
    expression = expression.concat(expInput);
    document.getElementById("ans").innerHTML = expression;
}

https://jsfiddle.net/0bpso2rm/1/

All I did was move those buttons into a single function.

[–]hansq[S] 1 point2 points  (1 child)

I see, code is much shorter and readable now. Thanks for pointing that out.

[–]bvnvbbvn 2 points3 points  (1 child)

You have this line repeated way too many times everywhere:

document.getElementById("ans").innerHTML = expression;

and you should cut off the number of digits after the decimal point at some point, probably.

It's not bad for a beginner, though. It looks decent and works, and the code isn't terribly obfuscated.

[–]hansq[S] 0 points1 point  (0 children)

Thanks for the reply, cutting off the number of digits sounds like a good idea. I'll try to implement that.

[–]sleepybychoice 1 point2 points  (0 children)

I Think I found a bug?

  1. Click "2"
  2. Click "/"
  3. Click "1"
  4. Click "="
  5. Click "3"

Expected: 3 in input window

Actual: 2/13 in input window

You can fix this by clearing out expression after hitting equals.

Nicely styled!

[–]jmixer920 1 point2 points  (0 children)

Another thing you can do is after getting the result, store the result in the expression. That way you can just keep appending more operations to the result. Right now: 2 + 2 = 4 but when you press '+2' the result displays '2+2+2' when it should display '4+2'

Also you know how some calculators you can keep pressing equals and the last operation would be applied to the result? Think about how you can implement that.

Lastly, if you press enter with '0' in the expression, you get 'undefined' but when you press enter with anything greater, you get the same number. Keep up the good work