Розробка · 15 травня 2019, 16:00 1343

Code review JavaScript

Доброго дня!

Для тих, хто має небагато вільного часу чи можете провести code review по моїх невеликих проектах і тестових завданнях.
github.com/Aleksandr-Protsak

Теми: code review, JavaScript

👍ПодобаєтьсяСподобалось0

До обраногоВ обраному0

Facebook

Twitter

Ctrl + Enter

Ваня 16.05.2019 19:21

це просто нечитабельно

@media (max-width: 1900px){.col{font-size: 1.5rem;}}

@media (max-width: 960px){
    .col{font-size: 1rem;}
    .row_val{height: 50px;}
    .user_content{flex-direction: column;}
    .width_ui{margin-left: 120px;}
    .ui_content{font-size: 0.7rem;}
    .form_row{flex-direction: column;}
    option{font-size: 0.7rem;}
}

є така хороша річ, як не заливати apiKeys і всякі такі доступи на гітхаб, тим більш в паблік репо
де package.json і всяка ця фігня в репозитрію з vue?

логіку івентів краще виносити в функції, її не має бути в темплейті!!!
навіщо 2 компоненти для button? можливо одним можна обійтись?
форматування не очень, є такі шутки як eslint/pretier, і не бійтеся використовувати пробіли, щоб виділити ті чи інші конструкції коду, за них не потрібно платити

alert — для чого?
address!!!

є такий охфігенний тег як label, було б краще їх використовувати замість span-ів
купа інпутів які фактично однакові і викидають івенти, можна якось це простіше значно зробити!!!
як вже писали doSome? WTF??

select? input? функції які слухають івенти мають починатись з on onSelect/onInput

’ui-button’ -> uiButton в js ’ui-button’ в темплейті, кебаб кейс в js зазвичай не використовується

якщо використовуєте стору, то в компонентах не має бути запитів до сервера, це має бути сховано у випадку vuex в actions
document.forms -> це краще якось інакше було б робити без напряму стукатись до dom

Відповісти

Підтримати

Aleksandr Protsak 16.05.2019 20:50

Щодо label, я знаю, але то дуже довга історія... а за решту дякую за детальний аналіз, врахую

Відповісти

Підтримати

Ваня

Ваня 16.05.2019 19:02

Timer
нафіга там jQuery ну або якщо що можна заюзати вже cdn-ку
почитайте про BEM, про порядо властивостей коли пишете css і на всякий випадко про те як браузер парсить селектори в css також не завадить

function pad -> є така функція як padLeft в js
можливо то тільки моє, але стільки id-шок то просто виглядає як вирвати очі
форматування в setInterval хєрове якесь
в js зазвичай використовуються ’’, а "" для html

зазвичай якщо значення не змінюється, то то const
якщо використовуєте обєкт, то краще вже клас заюзати і всяке таке, а то виходить і ніби в сторону класів і тд, а ніби і як функції, hh/mm/ss взагалі поза класом і якось дивно трішки обновлюються, і при цьому використовуються в класі, таке краще не робити

Відповісти

Підтримати

Aleksandr Protsak 16.05.2019 21:01

за cdn не чув прочитаю, якщо не важко пояснити «форматування в setInterval хєрове якесь» що саме не так?

Відповісти

Підтримати

Ваня

Ваня 16.05.2019 21:29

у вас

this.interval = setInterval( () => {
if(secTime === 0){
    this.reset();
    return alert('The end')
}
secTime -= 1;
    $("#hour").text(pad(Math.floor(secTime / 3600))); 
    $("#minute").text(pad(Math.floor(secTime / 60 % 60)));
    $("#second").text(pad(parseInt(secTime % 60)));
}, 1000);

мало б бути

this.interval = setInterval(() => {
    if (secTime === 0) {
        this.reset();
        alert('The end');
        return;
    }
    secTime -= 1;
    $("#hour").text( pad(Math.floor(secTime / 3600)) ); 
    $("#minute").text( pad(Math.floor(secTime / 60 % 60)) );
    $("#second").text( pad(parseInt(secTime % 60)) );
}, 1000);

і взагалі по коду vue багато питань до форматування також, спробуйте почитати можливо різні code conventions наприклад google/airbnb

Відповісти

Підтримати

Aleksandr Protsak

Aleksandr Protsak 16.05.2019 22:25

vue — перший фреймворк з яким довелося ознайомитися в короткі сроки і на якому треба було написати ТЗ тому я й не сумніваюсь що там питань багато. Ну і форматуванню не приділяв багато уваги, тепер буду знати.

Відповісти

Підтримати

Ваня

Ваня 17.05.2019 09:32

Форматування доволі важлива річ, так як код не тільки пишеться, а і читається, і часто читається частіше значно, особливо коли великий проект, і хотілося б при цьому розбиратися з логікою, а не рівнями вкладеності і взагалі що до чого відноситься. Я б порадив налаштувати eslint з якимись правилами пожорсткіше, через місяць навіть не будете помічати, що завжди пишите акуратний код.ну і завжди можна використовувати різні форматери, які то роблять замість тебе

Відповісти

Підтримати

Aleksandr Protsak

Ваня 16.05.2019 21:32

також порадив би розібратись трішки з less/scss

Відповісти

Підтримати

Aleksandr Protsak

Руслан Сірко 16.05.2019 17:59

github.com/...mer/blob/master/script.js

for (let i = 0; i < 60; i++) {
    if (i < 24) {
        $('#selected_hour').append($(`<option>${pad(i)}</option>`));
    }
    $('#selected_minute').append($(`<option>${pad(i)}</option>`));
    $('#selected_second').append($(`<option>${pad(i)}</option>`));
}

Відповісти

Підтримати