Solution#24
Conversation
| @@ -0,0 +1,74 @@ | |||
| let display=document.querySelector('.display') | |||
| let x,y; | |||
There was a problem hiding this comment.
Can you think of better names for it then x and y?
Your variable names should indicate what you mean
| let multiply = document.getElementById('multiply') | ||
| let divide = document.getElementById('divide') | ||
|
|
||
| plus.addEventListener('click', function(){ |
There was a problem hiding this comment.
If you will add some common class to your operators you could have array of elements that you will loop and apply the event listener - its a bit shorter
| let divide = document.getElementById('divide') | ||
|
|
||
| plus.addEventListener('click', function(){ | ||
| operator='+'; |
There was a problem hiding this comment.
Hardcoded strings should be saved inside constants
|
|
||
| let equal = document.querySelector('#equal') | ||
|
|
||
| let num = document.querySelectorAll('.number').forEach(btn=>{ |
There was a problem hiding this comment.
This is good practice, in my above comment i referred to something like this
| let num = document.querySelectorAll('.number').forEach(btn=>{ | ||
| btn.addEventListener('click', function(){ | ||
| if(!operator){ | ||
| x=Number(btn.textContent); |
There was a problem hiding this comment.
This could be NAN I would add a check for it
| x=Number(btn.textContent); | ||
| display.textContent=btn.textContent; | ||
| } | ||
| else{ |
There was a problem hiding this comment.
If you will add a return statement above here, you will not need the else part
| result=x+y; | ||
| display.textContent=result; | ||
| } | ||
| if(operator=='-'){ |
There was a problem hiding this comment.
Use else it to avoid checking all 4 of those checks every time
| } | ||
| if(operator=='/'){ | ||
| result=x/y; | ||
| display.textContent=result; |
There was a problem hiding this comment.
Maybe use it after all the ifs? and do it in 1 line and not 4 like you have in here
2 participants
No description provided.