Как улучшить мою проверку в JavaScript?
А именно, мне интересно, можно ли сократить, улучшить или вообще удалить некоторые части кода.
Я не хочу менять синтаксис ES5, потому что пока я не знаком с синтаксисом Es6.
Я также хочу избегать использования jQuery.
Это мой код проверки: https://codepen.io/RobotHabanera/pen/NWbKJLe
var form = document.querySelector('form');
var errormessage = document.querySelector('.error-message');
form.addEventListener('submit', function(e) {
e.preventDefault();
// musimy zalożyć czy formularz jest poprawnie wypełniony czy nie
var errors = [];
errormessage.innerHTML = '';
var clearAllParagraphsAtTheStart = document.querySelectorAll('.form-group p');
clearAllParagraphsAtTheStart.forEach(function(item) {
item.innerHTML = '';
});
// sprawdzić czy walidacja bedzie działać
for (var i = 0; i < form.elements.length; i++) {
var field = form.elements[i];
switch (field.name) {
case 'email':
// najpierw negujemy funkcje hasMonkeyInFiled i jesli negacja przebiegnie prawidłowo to wykona się prawa strona komunikatu && bo
// jesli negacja nie przebiegnie prawidłowo to linijka z prawej sterony sie nie wykona.
!hasMonkeyInFiled(field) && errors.push({
email: 'Email musi posiadać znak @'
});
break;
case 'name':
!hasMoreThanTwoChars(field) && errors.push({
name: 'Twoje imię jest za krótkie'
});
break;
case 'surname':
!hasMoreThanSixChars(field) && errors.push({
surname: 'Twoje nazwisko jest za krótkie'
});
break;
case 'pass1':
!hasCorrectPassword(field, form.elements[i + 1]) && errors.push({
pass1: 'Hasła nie są takie same lub puste'
});
break;
case 'pass2':
!hasCorrectPassword(field, form.elements[i - 1]) && errors.push({
pass2: 'Hasła nie są takie same lub puste'
});
break;
case 'agree':
!isChecked(field) && errors.push({
agree: 'Musisz zaakceptować warunki'
});
break;
}
}
if (errors.length) {
e.preventDefault();
errors = errors.filter(function(v, i, a) {
return a.indexOf(v) === i;
});
errors.forEach(function(item, index) {
// wklej wartość klucza z tablicy errors gdzie nazwa klucza jest równa wartości danego data-validation bierzącego inputa z pętli
var currentKey = Object.keys(errors[index]);
var currentValue = Object.values(errors[index]);
var inputs = document.querySelectorAll('.form-group input');
inputs.forEach(function(item, index) {
if (item.dataset.validation == currentKey) {
var elementP = document.createElement('p');
elementP.innerHTML = currentValue;
item.after(elementP);
}
});
/* errormessage.append(elementP); */
});
}
});
function hasCorrectPassword(field1, field2) {
if (hasMatch(field1, field2) && hasNumberChar(field1) && hasNumberChar(field1)) {
return true;
}
return false;
}
// paramertr 'field' to jest input element
function hasMonkeyInFiled(field) {
return field.value.indexOf('@') > -1;
}
// input ma wiecej niz 6 znakow
function hasMoreThanSixChars(field) {
return field.value.length > 6;
}
// input ma wiecej niz 2 znaki
function hasMoreThanTwoChars(field) {
return field.value.length > 2;
}
// checkbox musi byc zaznaczony
function isChecked(field) {
return field.checked;
}
// pierwsze i drugie hasło są identyczne ale nie puste
function hasMatch(field1, field2) {
// jak sprawdzic czy nie sa puste
if (field1.value.length && field1.value.length) {
return field1.value == field2.value;
}
return false;
}
// Warunek dla chętnych. Dodatkowe. Hasło ma mieć co najmniej 6 znaków (w tym co najmniej jedną liczbę i jedną literę)
function hasNumberChar(field) {
var numbers = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9];
var hasNumber = false;
numbers.forEach(function(number) {
if (field.value.indexOf(number) > -1) {
hasNumber = true;
}
});
return hasNumber;
}
function hasLetterChar(field) {
var chars = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm', 'n', 'o', 'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z'];
var hasChar = false;
chars.forEach(function(char) {
if (field.value.toLowerCase().indexOf(chars) > -1) {
hasChar = true;
}
});
return hasChar;
}
// input ma wiecej niz 5 znakow
function hasMoreThanFiveChars(field) {
return field.value.length > 5;
}
function isPangram(string) {
string.replace(/ /g, '');
for (var i = 0; i < string.trim().length; i++) {
var array = [];
var chars = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm', 'n', 'o', 'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z'];
chars.forEach(function(char) {
if (char == string.substr(string[i], 1)) {} else {
array.push(string[i])
}
});
}
if (array.length) {}
// bierze pierwszą litere z tablicy i jedzie tą literą po wszystkich znakach tekstu
string.replace(/ /g, '');
chars.forEach(function(char) {
for (var i = 0; i < string.length; i++) {
var chars = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm', 'n', 'o', 'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z'];
if (char == string[i]) {
console.log('jest');
} else {
console.log('nie ma');
}
}
});
};
function isPangram(string) {
string.replace(/ /g, '');
var array = [];
var chars = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm', 'n', 'o', 'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z'];
chars.forEach(function(char) {
for (var i = 0; i < string.length; i++) {
// każda litera alfabetu ma przejechać po tekście
if (char == string[i]) {
array.push(string[i]);
} else {}
}
if (array.length > 0) {
console.log('jest')
}
});
};
1 ответ
Функциональность валидации
В целом функция проверки выглядит разумной. У тебя есть preventDefault
вначале это блокирует отправку формы, но в остальном она выглядит логически структурированной. Вы сначала очищаете все свои предыдущие ошибки, затем смотрите, есть ли новые ошибки, и, наконец, показываете новые ошибки. Вы можете рассмотреть возможность выделения этих трех частей, но я не думаю, что это необходимо в данном конкретном случае.
Однако есть некоторые проблемы с функциями, которые вы вызываете, и некоторые ложные предположения, которые вы делаете внутри и вокруг них.
hasMoreThanTwoChars
Когда вы обнаруживаете, что пишете числа внутри имен функций, это обычно означает, что вы делаете что-то не так. Есть либо лучшее описание (например, fibonacci
вместо того addLastTwoItemsOfSequence
) или в вашем случае вы можете и должны вынести число из параметра.
В вашем случае вы можете написать функцию hasMinimumNumberOfCharacters(field, numberOfCharacters)
который обрабатывает все три варианта.
name
и surname
Хотя следующей статье более десяти лет, она все еще очень применима: Программисты верят в ложь об именах. Например, я не могу заполнить вашу форму, не дополнив свое имя пробелами или чем-то подобным.
hasCorrectPassword
Вы называете свой hasCorrectPassword
функционировать с form.elements[i - 1]
и form.elements[i + 1]
. Вам почти гарантировано, что в какой-то момент это сломается из-за переупорядочения или добавления полей. Сделайте его более надежным, ссылаясь на сами поля по имени, например, через document.querySelector('#pass2')
.
Кроме того, сама функция нечетная, поскольку выполняет две функции. Вам, вероятно, следует разделить его на две четко определенные функции. Один isEqualToField(field1, field2)
и один containsNumber(field)
. Если ваши требования к паролю станут более сложными, вы можете вместо этого рассмотреть isValidPassword(field)
и isEqualToField(field1, field2)
.
Вы также, кажется, дважды проверяете, есть ли в одном и том же поле числовой символ.
И, наконец, если у вас есть инструкция if-else, которая либо возвращает true, либо false, вы можете просто вернуть условие самого оператора if-else.
function hasCorrectPassword(field1, field2) {
return hasMatch(field1, field2) && hasNumberChar(field1);
}
hasMatch
Всегда проверяйте равенство, используя ===
. Всегда помните о типах переменных и никогда не полагайтесь на автоматическое преобразование типов для работы вашего кода.
hasNumberChar
Эту функцию можно улучшить разными способами. numbers
это массив Number
, и вы полагаетесь на String.indexOf
чтобы преобразовать элементы в строку. Вы можете сделать его массивом String, чтобы быть более внимательным, когда вы используете String и когда вы используете Number. Кроме того, не очень хорошо определять белый список символов, но я полагаю, что мы не будем использовать какие-либо другие цифры, кроме тех 10, которые мы здесь определили.
Вместо того Array.forEach
который требует, чтобы вы перебрали все элементы, вы можете использовать несколько других методов, которые позволят вам вернуться раньше.
Array.some(func)
и Array.every(func)
вернуть логическое значение, если соответственно один или все элементы в массиве соответствуют условию, и вернуться раньше. Вы бы переписали это как
digits.some(function (digit) {
// This string contains the digit we are looking for
return field.value.indexOf(digit) !== -1;
});
Вы можете переписать forEach
петли с for(const x of myArray)
а также с дополнительным преимуществом, заключающимся в том, что вы можете вернуться из этого цикла или выйти из него.
for (const digit of digits) {
if (field.value.indexOf(digit) > -1) {
return true;
}
}
return false;
Оба этих подхода имеют дополнительное преимущество, которое вы не отслеживаете. hasNumbers
, но вернитесь немедленно, когда узнаете ответ.
hasLetterChar
Эта функция имеет те же проблемы, что и предыдущая, за исключением того, что у нас есть еще одна проблема. Ваш язык содержит много символов, которые не соответствуют вашему белому списку, и использование этого белого списка вызовет проблемы, по крайней мере, для части ваших пользователей. Даже в языках, где символы вне этого белого списка необычны или отсутствуют, не следует полагаться на такой белый список. Чтобы узнать, почему, я снова вернусь к статье несколькими разделами выше.
Если вы решите оставить его вообще, подумайте, достаточно ли проверки, содержит ли он что-нибудь, кроме пробелов (value.match(/S/)
). В противном случае посмотрите на что-то вроде этот вопрос о Stackoverflow, но имейте в виду, что вы, скорее всего, пропустите целые скрипты, и люди будут раздражены тем, что не могут ввести свое имя из-за чрезмерно ограничивающих средств проверки.
isPangram
Функция isPangram
определяется дважды и дважды не возвращает логическое значение. Поскольку он не служит никакой цели, его следует удалить.
Обе функции содержат странные вещи, такие как пустые операторы if или else, такие проблемы, как неиспользование строгого равенства ===
как упоминалось ранее, и нечетный белый список.
Использовать let
вместо того var
Даже если вы не используете ничего, кроме этого, из современного javascript, используйте let
при объявлении переменных вместо var
. Переменные, объявленные с помощью var
могут быть прочитаны и записаны вне контекста, в котором вы ожидаете их использования. Примером этого является переменные, определенные с помощью var внутри цикла, доступны вне этого цикла. С помощью let
волшебным образом исправит это, будучи доступным для чтения и записи только после того, как вы его определите, и должным образом отбрасывая его в конце блока, в котором он определен. Некоторые подробные сведения об области видимости можно найти в Каков объем переменных в Javascript.
function testWithLet() {
// console.log(x); // Uncaught ReferenceError: x is not defined
// x += 5; // Uncaught ReferenceError: x is not defined
for (let i = 0; i < 5; i++) {
const x = 1;
}
// console.log(i); // Uncaught ReferenceError: i is not defined
}
function testWithVar() {
console.log(x); // undefined
x += 5; // What did this just do?
for (var i = 0; i < 5; i++) {
var x = 1;
}
console.log(i); // 5
}
testWithLet();
testWithVar();
Некоторые дополнительные примечания на основе кода
Имейте в виду, что в браузере уже есть встроенные функции как для проверки, так и для упрощения заполнения полей формы. В частности, используйте type
определять такие вещи, как email
, tel
или number
. Вы можете (но не обязаны) определять шаблоны, которых должен придерживаться ввод. (документы на mdn)
Использовать inputmode
, чтобы указать браузеру, какую экранную клавиатуру им следует использовать. Это особенно удобно для быстрого большинства людей, которые используют мобильное устройство для использования формы. Допустимые значения включают text
, tel
, email
и url
. (документы на mdn)
Ярлыки должны быть связаны с их полями формы. Либо используйте <label>label text: <input /></label>
или for=".."
на этикетке, чтобы связать его с входом. Вы делаете это, чтобы люди с программами чтения с экрана имели представление о том, на что они смотрят, и чтобы люди могли выбирать поле, щелкая по ярлыку.
Вау, хороший рабочий человек, я внимательно прочитаю его вскоре, чтобы поделиться своими мыслями о том, что ты написал, спасибо
— Петр
Вы заявляете в отношении
var
«… не очень хорошо следите за областью видимости» что неверно, и «… наверх …» должны быть «в верхней части функции или глобальной области видимости». Ваши утверждения об использованииlet
просто доктрина. Предоставляется точная информация, если ее слишком много писать, дайте ссылки. Я не буду голосовать против, так как остальная часть ответа хороша (должна была быть принята)— Слепой67
Я не согласен с вашим утверждением, но я немного изменил формулировку, добавил ссылки и добавил пример. Я полагаю, вы могли видеть это
var
очень хорошо следует функции / глобальной области видимости, если вы принимаете во внимание подъем на вершину этой функции, но для людей, не очень хорошо знакомых с причудами переменных javascript, объявленных с помощьюvar
просто делай «странное дерьмо». И есть нет реальной причины использовать это больше.— Сумурай8