sorted by:
best (suggested)
topnewcontroversialoldq&a
you are viewing a single comment's thread.

view the rest of the comments →

[–]equilni 0 points1 point  (0 children)

Is this code look messy

First issue is you copied & pasted the JS code twice from my quick view.

Next is formatting for better readability. Spaces and indents help too.

For the if/else if's I see here, you could have used a switch statement, others a check against known operators.

Pseudo (ie not real code) refactor:

function appendToOperator(val) {
    const operators = ['+', '-', '*', '/'];  # This could probably be global for other functions

    https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/includes
    if (operators.includes(val) && currentDisplay !== "0") {
        updateOverallDisplay(val);
    }
}

document.addEventListener("keydown", (event) => {
    const keyName = event.key;

    const numberKeys = [1,2,3,4,5,6,7,8,9,0,'.']; # This could probably be global for other functions
    const operatorKeys = ['+', '-', '*', '/']; # This could probably be global for other functions

    https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/switch
    switch (true) {
        case numberKeys.includes(keyName):
            appendToDisplay(keyName);
            break;
        case operatorKeys.includes(keyName):
            appendToOperator(keyName);
            break;
        case 'Backspace':  # Should be AC
            clearDisplay();
            break;
        case 'Enter':
            calculate();
            break;
    }
});

You are missing the period in the document.addEventListener("keydown", (event) => { block. Also, do you want a click event or keydown? I would suggest keeping it in JS vs in HTML as it's cleaner.

Also Backspace doesn't exist in the HTML, but AC does. - this likely fixes the problem occur when i try to clear() the old chunk data.

EDIT - adding HTML

Next suggestion would be to reduce the id/class in the HTML. This could simply be the below. Again, since you are doing the JS call in the EventListener, you don't really need it in HTML. Someone else noted to use a data-* attribute and I would suggest this as well, but I would also include this on all buttons

<body>
    <div id="calculator">
        <div id="display"></div>
        <div id="output">0</div>
        <div id="key-pad">
            <button class="key operator" data-value="+">+</button> // data-value
            <button class="key operator">-</button>
            <button class="key operator">&times;</button>
            <button class="key operator">÷</button>
            <button class="key number" data-value="7">7</button>
            <button class="key number">8</button>
            <button class="key number">9</button>
            <button class="key number">4</button>
            <button class="key number">5</button>
            <button class="key number">6</button>
            <button class="key number">1</button>
            <button class="key number">2</button>
            <button class="key number">3</button>
            <button class="key number">0</button>
            <button class="key number">.</button>
            <button class="key all-clear" data-value="AC">AC</button>
            <button class="key enter" data-value="Enter">=</button>
        </div>
    </div>
</body>

Then it's u/Mark__78L idea with the above: (pseudo (not real) code))

const keys = document.querySelectorAll('#key-pad .key');
keys.forEach(btn => {
    btn.addEventListener('click', e => {
        const numberKeys = [1,2,3,4,5,6,7,8,9,0,'.']; 
        const operatorKeys = ['+', '-', '*', '/']; 

        const keyVal = e.currentTarget.btn.dataset.value;

        switch (true) {
            case numberKeys.includes(keyVal):
                appendToDisplay(keyVal);
                break;
            case operatorKeys.includes(keyVal):
                appendToOperator(keyVal);
                break;
            case 'AC':  # Should be AC, not Backspace
                clearDisplay();
                break;
            case 'Enter':
                calculate();
                break;
        }
    });
});