search

iricea / iricea.github.io

0 stars 2 forks source link

Lesson 10-11 #5

Closed jeron-diovis closed 8 years ago

jeron-diovis commented 8 years ago

Замечания:

Lesson 10

https://github.com/iricea/iricea.github.io/blob/9f2edc6051d085e8f7b31f39dc9fd24ece9da2b5/GoIT%2Fjs%2Flesson10-11%2Fjs%2Fscript.js#L7-L10

Форматирование совершенно неприемлемо. Смотри первую ссылку из ДЗ. С форматирования код начинается, и чтоб его хотелось читать, он должен аккуратно выглядеть)

https://github.com/iricea/iricea.github.io/blob/9f2edc6051d085e8f7b31f39dc9fd24ece9da2b5/GoIT%2Fjs%2Flesson10-11%2Fjs%2Fscript.js#L3-L4

Конвертацию в число через + Олег в видео показывал, ок. Но вообще, это скорее своеобразный хак, нежели общепринятая практика. Такое можно встретить в библиотеках, например, но в бизнес-коде обычно так не принято (по крайней мере, я не особо встречал). Для этих целей существует функция parseInt (и parseFloat для дробных). Результат будет тот же, но сам код более семантичный, более читабельный. Это хороший тон.

Опять же, форматирование - пробел после плюса лишний. Унарные операторы принято писать слитно.

Lesson 11

https://github.com/iricea/iricea.github.io/blob/9f2edc6051d085e8f7b31f39dc9fd24ece9da2b5/GoIT%2Fjs%2Flesson10-11%2Fjs%2Fscript2.js#L4

Об осмысленном именовании переменных говорилось в видео. Рядом есть переменная name, почему бы массив, соответственно, не назвать names?

https://github.com/iricea/iricea.github.io/blob/9f2edc6051d085e8f7b31f39dc9fd24ece9da2b5/GoIT%2Fjs%2Flesson10-11%2Fjs%2Fscript2.js#L6 https://github.com/iricea/iricea.github.io/blob/9f2edc6051d085e8f7b31f39dc9fd24ece9da2b5/GoIT%2Fjs%2Flesson10-11%2Fjs%2Fscript2.js#L16

Ошибки оформления. Найди с трёх попыток.

https://github.com/iricea/iricea.github.io/blob/9f2edc6051d085e8f7b31f39dc9fd24ece9da2b5/GoIT%2Fjs%2Flesson10-11%2Fjs%2Fscript2.js#L7-L8

Два момента. Во-первых, переменная name избыточна. Она получает значение, записывается в массив, и больше ни для чего не используется. Вместо этого можно сразу записать значение в массив и убрать name совсем. Если бы с введённым именем проводились какие-то ещё манипуляции перед тем, как в массив положить - какая-то валидация, замена символов etc. - это было бы ок. А так - избыточно.

Во-вторых. Добавление элементов в массив через присвоение по индексу - плохая практика. В маленьком примере это, конечно, несущественно, но следует помнить и понимать, что на больших массивах такой подход может работать весьма медленно. В видеоуроках показан метод массива push - для добавления элемента в конец массива всегда следует использовать именно его.

Дополнительно:

https://github.com/iricea/iricea.github.io/blob/9f2edc6051d085e8f7b31f39dc9fd24ece9da2b5/GoIT%2Fjs%2Flesson10-11%2Fjs%2Fscript.js#L9

Самое время использовать сокращённую запись. Там где можно, её всегда стоит использовать - опять же, код чище и читабельнее. "Увеличить переменную вдвое" понятнее, чем "записать в переменную результат умножения её текущего значения на два".

https://github.com/iricea/iricea.github.io/blob/9f2edc6051d085e8f7b31f39dc9fd24ece9da2b5/GoIT%2Fjs%2Flesson10-11%2Fjs%2Fscript.js#L15

Тут всё в порядке, но в качестве бонуса можешь найти информацию про string substitutions в консоли и применить её в этой строке.

https://github.com/iricea/iricea.github.io/blob/9f2edc6051d085e8f7b31f39dc9fd24ece9da2b5/GoIT%2Fjs%2Flesson10-11%2Fjs%2Fscript2.js#L13

Вообще, за indexOf плюсик. Это правильно. Если ты знаешь, что есть функция, которая сделает работу за тебя - используй её. Но в задании сказано "введенное имя циклом сравнивать с именами в массиве" - думаю, чтобы проверить, напишете ли вы этот самый цикл оптимально или "в лоб". Это имеет смысл проверить - поэтому сделай цикл, пожалуйста.

jeron-diovis commented 8 years ago 
for (var i = 0; i < names.length; i++) {
    var check = 0;
    check += (yourName === names[i]);
}
...
if (check > 0) { ...

Рассуждаем.

Нужно определить, присутствует ли элемент в массиве - не подсчитать количество вхождений, а сказать - да или нет.

Следовательно - достаточно булевой переменной вместо счётчика.

И назвать её стоит не check - это довольно-таки безлико - а, например, hasMatch. Булевые переменные принято именовать подобным образом: isSomething, hasSomething, containsSomething, и так далее, по смыслу. Такое негласное соглашение здорово помогает при чтении, т.к. ты сразу понимаешь, чего от этой переменной можно ожидать.

Далее. Раз нам не нужно подсчитывать количество вхождений - значит, найдя первое, дальше по циклу идти нет смысла, это просто никому не нужно. Найди информацию, как прервать выполнение цикла, поправь - и будет хорошо.

jeron-diovis commented 8 years ago

Зачёт.