Как улучшить код javaScript и подход в целом?

Здравствуйте!
Я начал изучать JavaScript не так давно и в рамках собственного учебного проекта написал музыкальный секвенсор используя чистый javascript и библиотеку Tone.js. Попробовать результат можно тут https://fedorpereverzev.github.io/MusicSeq.github.io/ .
К сожалению у меня нет ментора, который мог бы посмотреть код, поэтому я обращаюсь к вам.
Не могли бы вы глянуть код https://github.com/FedorPereverzev/MusicSeq.github.io и указать на ошибки/направление для улучшения?

(Я не занимался адаптацией css под все браузеры итд. Корректно отображается в хроме и сафари. Главной целью ставил для себя рабочий функционал. )

Спасибо!
  • Вопрос задан
  • 481 просмотр
Решения вопроса 2
@daniil14056
Уделил час времени, вот основные недочеты, будет приятно если ознакомитесь.
1.!!!!!! Много обработчиков. Можно одним все решить. Почитайте про делегирование событий.
///
var interface=document.getElementById("interface");
interface..addEventListener('input', function (e) {
        var target= e.target || e.srcElement // получаем элемент где произошло событие
        switch(target.id){    
              case "hatHarmon" : // для каждого input
                      aVolume = e.target.value;
                       break;
               case  //.....
    });
// И проще и быстрее и нагляднее. Вместо мусора ...Button-ов то же самое одним все накрыть.


2.Слишком много document.querySelector ты каждый раз во всем document, то есть во всем dom дереве ищешь то что находиться рядом! Намного быстрее будет
var interface=document.getElementById("interface"); 
var hatValume=interface.querySelector("#hatVolume")
tomValue=hatValume.parentElement.nextElementSibling.querySelector("#tomVolume");

3. getDocumentId быстрее чем querySelector(#id) в разы, сам проверял.

4. Область видимости ;(function(){ ю..весь код...})();

5. У тебя ++i в конце в цикле script.js не обрабатывает первый элемент.. Замени на i++;
Т.е. ты находишь элемент и уже в нем ищешь.

6.
if (dCh.checked) {
        dVel = 1;
    };
    
    if (!dCh.checked) { // зачем, лишние операции, потеря скорости,  замени на else
        dVel = 0.5;
    };

    if (cCh.checked) {
        cVel = 1;
    };
    // не красиво долго, плохие имена, я путаю, "dCh cCh - 10 сек уходит на нахождение различия". Код не поддерживается, долго искать что такое cCh, вся область засорена мусорными глобальными переменными. 
// Решение, разбить все на блоки или функции, или Паттерн реализовать, к примеру фабрику для звуков и акцентов
// Массивы у вас не массивы а куча переменных, можно автоматизировать циклом, создать массив из этих элементов, и перебирая его уже составить 2 массива из звуков и акцентов, и уже работать с ними.

7. Не правильно именуете переменные, переменные с маленькой буквы именуют, а классы большими.

8style.css очень плохо, вы снова каждый раз ищите во всем документе. Составляйте более конкретные селекторы
#start:hover,  
#stop:hover,
#clear:hover {
    background-color: #2C7769;
}
/* медленно, xQuery долго(для больших проектов конечно) их ищет этот селектор. Нужно расписать до него путь поконкретнее*/
#playStop div{
       background-color: #2C7769;
}


9 Вы много раз в подряд объявляете i, let i не спасает в циклах, у вас есть let i=0; глобальной области в начале где-то. Потом можно уже просто for(i=0;i<....
Вроде все. Но код рабочий конечно, но почти не живучий.
Ответ написан
@alfangur
1. тебе нужно весь код из script.js завернуть в (function(){/*your code*/})(), правильного термина к такой функции не знаю. данным маневр нужен для того что бы твой код не "гадил" в глобальном пространстве.
2. в своем скрипте первой строчкой ты обращаешься к объекту сторонней библиотеки. в таких случаях я всегда проверяю наличие необходимого объекта.
Ответ написан
Пригласить эксперта
Ваш ответ на вопрос

Войдите, чтобы написать ответ

Похожие вопросы