Review kodu javascript i optymalizacja

Question

Witam. Mam już skończony swój timer w javascript i chciałbym prosić o przegląd kodu, opinię, informację na temat ewentualnych poprawek bądź optymalizacji kodu

Kod javascript zamieszczam na forum w bloczku

const timer = document.querySelector('.timer');
const hours = document.querySelector('#timer-hours');
const minutes = document.querySelector('#timer-minutes');
const seconds = document.querySelector('#timer-seconds');
const alarm = new Audio ('timer.mp3');
let countDownIsOn = false;

function formatNumbers(e)
{
    e.preventDefault();
    const span = e.target;
    if (e.code == 'Delete')
    {
        span.textContent == '00';
        return;
    }

    if (/^\d$/.test(e.key))
    {
        const currentValue = span.textContent, inputValue = e.key;

        let newValue = parseInt(currentValue + inputValue);
        if (newValue > 99) newValue = newValue.toString().slice(-2);
        if (newValue > 59) newValue = inputValue;
        span.textContent = ('00' + newValue).slice(-2);
    }
}

function countDown()
{
    if (hours.textContent == 0 && minutes.textContent == 0 && seconds.textContent == 0)
    {
        alarm.play();
    }
    else
    {
        setTimeout('countDown()', 1000);
        hours.setAttribute('contenteditable', 'false');
        minutes.setAttribute('contenteditable', 'false');
        seconds.setAttribute('contenteditable', 'false');
        if (seconds.textContent>0)
        {
            seconds.textContent--;
            if (seconds.textContent < 10)
            {
                seconds.textContent = '0' + seconds.textContent;
            }
        }
        else
        {
            seconds.textContent = 59;
            if (minutes.textContent>0)
            {
                minutes.textContent--;
                if (minutes.textContent < 10)
                {
                    minutes.textContent = '0' + minutes.textContent;
                }
            }
            else
            {
                minutes.textContent = 59;
                if (hours.textContent>0)
                {
                    hours.textContent--;
                    if (hours.textContent < 10)
                    {
                        hours.textContent = '0' + hours.textContent;
                    }
                }
            }
        }
    }
}
timer.addEventListener('keydown', formatNumbers);
window.addEventListener('keydown', (event) => {
    event.preventDefault();
    if (event.key === 'Enter' && countDownIsOn == false)
    {
        countDownIsOn = true;
        countDown();
    }
    else if (event.key === 'Enter' && countDownIsOn == true)
    {
        alarm.pause();
    }
})

A całość na code pen

Answer 1

• Wszystko jest w globalnym scope'ie, do owrapowania.
• Jest słaba nomenklatura: formatNumbers brzmi jakby przyjmował tablicę floatów/intów i zwracał tablice stringów w odpowiednim formacie. Tymczasem robi bardzo dużo rzeczy. countDown brzmi jak jakaś prosta funkcja, hook, a robi literalnie wszystko. minutes jak widzę w kodzie to zakładam że to number z liczbą minut. Tymczasem u Ciebie to element.
• Ja kiedy mam umieścić ifa w ifie, to zastanawiam się jak to napisać czytelniej, aby tego uniknąć. U Ciebie są 4 zagnieżdżone ify, kod jest strasznie nieczytelny.
• Masz niespójne formatowanie kodu, proponuję zainstalować eslinta. W jsie brackety otwierające są raczej w tej linijce, której dotyczą.
• Jest dość spory problem z architekturą tego. Proponuję oddzielić logikę od widoku. Niech logika w odpowiednich interwałach aktualizuje swój stan. Widok mając dostęp do tego stanu, niech się odświeża po logice. To chyba najbardziej trywialny podział, który umożliwi czytanie tego ze zrozumieniem.
• Proponuję żebyś wgl porzucił jsa, zdecydowana większość nowych projektów powstaje teraz w Typescriptcie.

Comments

Dzięki za wyczerpującą odpowiedź. Postaram się na to wszystko zwrócić uwagę