F
F
Fedor2018-03-07 19:22:47
JavaScript
Fedor, 2018-03-07 19:22:47

How to improve javaScript code and approach in general?

Hello!
I started learning JavaScript not too long ago and as part of my own learning project I wrote a music sequencer using pure javascript and the Tone.js library. You can try the result here https://fedorpereverzev.github.io/MusicSeq.github.io/ .
Unfortunately, I do not have a mentor who could look at the code, so I turn to you.
Could you take a look at the code https://github.com/FedorPereverzev/MusicSeq.github.io and point out any bugs/directions for improvement?
(I did not adapt css for all browsers, etc. It is displayed correctly in chrome and safari. The main goal was to set working functionality for myself.)
Thank you!

Answer the question

In order to leave comments, you need to log in

1 answer(s)
D
Daniel, 2018-03-08
@Seagul

I spent an hour of time, here are the main shortcomings, it will be nice if you read it.
one.!!!!!! Lots of processors. One can solve everything. Read about event delegation.

///
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. Too many document.querySelector every time in the entire document, that is, in the entire dom tree, you are looking for something that is nearby! It will be much faster
var interface=document.getElementById("interface"); 
var hatValume=interface.querySelector("#hatVolume")
tomValue=hatValume.parentElement.nextElementSibling.querySelector("#tomVolume");

3. getDocumentId is several times faster than querySelector(#id), I checked it myself.
4. Scope ;(function(){ y..all code...})();
5 . You have ++i at the end of the loop script.js does not process the first element. Replace with i++;
Those. you find an element and already look in it.
6 .
if (dCh.checked) {
        dVel = 1;
    };
    
    if (!dCh.checked) { // зачем, лишние операции, потеря скорости,  замени на else
        dVel = 0.5;
    };

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

7 . You do not name variables correctly, variables are named with a small letter, and classes are named with large ones.
8 style.css is really bad, you're looking all over the document again every time. Compose more specific selectors
#start:hover,  
#stop:hover,
#clear:hover {
    background-color: #2C7769;
}
/* медленно, xQuery долго(для больших проектов конечно) их ищет этот селектор. Нужно расписать до него путь поконкретнее*/
#playStop div{
       background-color: #2C7769;
}

9 You declare i many times in a row, let i does not save in cycles, you have let i=0; global scope at the beginning somewhere. Then you can just for(i=0;i<....
It seems like everything. But the code is working, of course, but almost not tenacious.

Didn't find what you were looking for?

Ask your question

Ask a Question

731 491 924 answers to any question