1
\$\begingroup\$

This code displays a simple decimal calculator with operations like add, subtract, multiply and divide. It also has a function to clear the result.

I try to reduce code repetition. I still think I used a lot of unnecessary if-else blocks here but tried my best. How can I reduce use of if-else blocks here?

<head>
    <meta charset="UTF-8">
    <meta http-equiv="X-UA-Compatible" content="IE=edge">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <link rel="stylesheet" href="assets/sass/main.css">
    <script src="https://kit.fontawesome.com/02234e3b3c.js" crossorigin="anonymous"></script>
    <title>JavaScript Calculator Project</title>
</head>
<body>
  <a class="view-github" href="https://github.com/yasinkarax/basicCalculator" target="_blank">
                VIEW ON GITHUB
            </a>
    <div class="container">
        <div id="screen">
            <div id="math">6000/2+3227*2</div>
            <div id="result">
                <span id="result-equal">=</span>
                <span id="result-value">12,454</span>
            </div>
        </div>
        <div class="buttons">
            <button class="clear operator"  id="clear">C</button>
            <button class="del operator" id="del">
                <i class="fa-solid fa-delete-left"></i>
            </button>
            <button id="div" class="div operator">/</button>
            <button id="multi" class="multi operator">*</button>
            <button class="seven digit">7</button>
            <button class="eight digit">8</button>
            <button class="nine digit">9</button>
            <button id="sub" class="min operator">-</button>
            <button class="four digit">4</button>
            <button class="five digit">5</button>
            <button class="six digit">6</button>
            <button id="add" class="add operator">+</button>
            <button class="one digit">1</button>
            <button class="two digit">2</button>
            <button class="three digit">3</button>
            <button id ="equal" class="equal operator">=</button>
            <button class="zero digit">0</button>
            <button id="dot" class="dot operator">.</button>
        </div>
    </div>
    <script src="assets/main.js"></script>
</body>


// variables
let 
math = document.querySelector('#math'),
value = document.querySelector('#result-value'),
buttons = document.querySelectorAll('button'),
clear = document.querySelector('#clear'),
del = document.querySelector('#del'),
equal = document.querySelector('#equal'),
buttonsL = buttons.length;


const 
operators = '+-*/.', 
err1 = "önce sayı girmelisiniz", 
empty = '';

document.onload = calculator();

function readyCal(){
    math.innerHTML = empty;
    value.innerHTML = empty;
}

//calculate the result
function calculate(){
        return Intl.NumberFormat('tr-TR').format(Function("return " + math.innerHTML)())
}

// returns the buttons's value
let buttonInner = i => buttons[i].innerHTML;


function calculator(){
    readyCal();
    for(let i = 0; i < buttonsL; i++){
        buttons[i].addEventListener('click', function (){
            if (math.innerHTML == empty && operators.includes(buttonInner(i)))
                math.innerHTML += err1;

            else if (operators.includes(math.innerHTML[math.innerHTML.length - 1])  && operators.includes(buttonInner(i)))
                math.innerHTML = math.innerHTML;

            else if (buttonInner(i) == clear.innerHTML)
                readyCal();


            else if (buttonInner(i) == equal.innerHTML)
                value.innerHTML = calculate();

            else if (buttonInner(i) == del.innerHTML)
                value.innerHTML = empty;
                
            else if(math.innerHTML == err1 && operators.includes(buttonInner(i)))
                    math.innerHTML =  err1;
            
            else if(math.innerHTML == err1 && operators.includes(buttonInner(i)) == false)
                math.innerHTML = buttonInner(i)

            else if(value.innerHTML != empty){
                    if(operators.includes(buttonInner(i))){
                        math.innerHTML = value.innerHTML.replaceAll('.', '') + buttonInner(i); 
                        value.innerHTML = empty;
                    }
                    else{
                        math.innerHTML = buttonInner(i);
                        value.innerHTML = empty;
                    }
            }
            else{
                math.innerHTML += buttonInner(i)
                
            }
        })
    }
}

here is codepen

screenshot of image

github repo

\$\endgroup\$
9
  • \$\begingroup\$ This code is already a working project. I asked how I can use less if else blocks in my projects in general, not this code specific. The code is already in a calculator code description. Do I have to explain everything down to the smallest detail? If you are aware, this is not a stackoverflow. \$\endgroup\$
    – Yasin
    Commented Mar 3, 2023 at 14:54
  • \$\begingroup\$ Code behind links is not reviewable; it needs to be included in the actual question. Thank you for fixing that. \$\endgroup\$ Commented Mar 3, 2023 at 16:06
  • \$\begingroup\$ @TobySpeight how it isn't reviewable? \$\endgroup\$
    – Yasin
    Commented Mar 3, 2023 at 16:09
  • \$\begingroup\$ Welcome to CR. I apologize that it has been challenging so far. Hopefully my edit to the title describes what the code does but feel free to update if need be. The code and image could be replaced with a runnable snippet. The link in Toby's comment mentions "Please embed the code to be reviewed directly in the question, for multiple reasons, including quality and licensing." \$\endgroup\$ Commented Mar 3, 2023 at 16:35
  • 1
    \$\begingroup\$ @Yasin Anything you post on Stack Exchange (code review and stack overflow) has a copyright and stack exchange licenses everything. I changed my down vote to an up vote, there is still one down vote. \$\endgroup\$
    – pacmaninbw
    Commented Mar 4, 2023 at 15:54

1 Answer 1

2
\$\begingroup\$

Your code works, but it's a bit unreadable due to all the if else statements. One problem is that all the buttons call the same function, causing a lot of checks to know which button was clicked. You can do two things to solve this:

  • Use the event object to know which button was clicked
  • Don't add the same handler to all the buttons

I put a little example here, for only the 7 and 8 buttons and the /, * and = operators. I didn't rewrite your whole calculator but hopefully it gives you some ideas.

HTML

<button id="div" class="div operator" data-op="/">/</button>
<button id="multi" class="multi operator" data-op="*">*</button>
<button class="seven digit" data-id="7">7</button>
<button class="eight digit" data-id="8">8</button>
<button id="equal" class="equal">=</button>

JS

const operatorbuttons = document.querySelectorAll('button.operator')
const digitbuttons = document.querySelectorAll('button.digit')
const equal = document.querySelector('#equal')

function calculator(){
    for(let btn of operatorbuttons){
        btn.addEventListener('click', (evt) => {
            if (math.innerHTML != empty) math.innerHTML += evt.target.dataset.op
        })
    }
    for(let btn of digitbuttons){
        btn.addEventListener('click', (evt) => {
            math.innerHTML += evt.target.dataset.id
        })
    }
    equal.addEventListener('click', () => {
      math.innerHTML = Intl.NumberFormat('tr-TR').format(Function("return " + math.innerHTML)())
    })
}
\$\endgroup\$
2
  • \$\begingroup\$ Thank you for feedback! I added unnecessary if-else blocks because I need to prevent double operator typing to the screen and if a result is calculated before the user clicks a button I need to write the new value with the result. It was the only way I found. \$\endgroup\$
    – Yasin
    Commented Mar 5, 2023 at 19:03
  • 1
    \$\begingroup\$ Yes, that check should be added again for the operator buttons. You might even look into regular expressions to see if a string has a correct order of numbers and operator symbols. \$\endgroup\$
    – Kokodoko
    Commented Mar 5, 2023 at 19:56

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.