Автор оригинала: Clint Winter.
У меня недавно у меня была ситуация на работе, где коллега пытался изменить функцию JavaScript, которую я написал, но завершил введение некоторых ошибок. При рассмотрении их кода казалось, что их проблема не полностью понимала, что делала функция, но я считаю, что это моя вина, потому что функция была, честно говоря, плохо написана.
Иногда у нас есть сроки, и, чтобы встретиться с ними, мы можем оставить вещи беспорядок. У меня были планы пересматривать его, но, конечно, другие вещи приняли приоритет. Теперь, когда функция вернулась на дверь, я увидел возможность исправить это.
Часто, когда мы разделяем наш код с миром, мы делимся нашим наиболее тщательно поддерживаемым материалом. Это не реальность бизнеса все время. В конце дня продукт и клиенты, которые используют это приоритет. Когда дело доходит до сроков против идеально чистого кода, крайний срок выигрывает. Тем не менее, когда мы получаем возможность вернуться и убирать после себя, мы должны принять эти возможности, потому что важно, чтобы мы балансировали производство с нашими возможностями для продолжения производства.
Я собираюсь попытаться исправить больную функцию в шагах, чтобы дать вам пример того, как я прошел процесс улучшения кода.
Оригинальный код
Давайте теперь посмотрим на оригинальную функцию, которая дала моим товарищам разработчика.
function valid(field, visibleField) {
var state = {
saved: true,
requirements: {
Description: {
required: true,
maxlength: 150
},
DueDate: {
date: true
},
PriorityID: {},
TypeID: {}
}
};
if (!state.requirements[field.name]) {
return true;
}
var errorField = visibleField ? visibleField : field;
// required
if (state.requirements[field.name].required) {
if (field.tagName.toLowerCase() == 'input' && field.value.length == 0) {
errorField.classList.add('inputBorderError');
return false;
} else if (field.value === undefined || field.value === '') {
errorField.classList.add('inputBorderError');
return false;
}
}
// max length
if (state.requirements[field.name].maxlength) {
if (field.value.length > state.requirements[field.name].maxlength) {
errorField.classList.add('inputBorderError');
return false;
}
}
// date
if (state.requirements[field.name].date) {
if (!moment(field.value, ['MM/DD/YYYY', 'YYYY-M-D'], true).isValid()) {
errorField.classList.add('inputBorderError');
return false;
}
}
errorField.classList.remove('inputBorderError');
return true;
}
Позвольте мне также предоставить некоторые упрощенные HTML, чтобы вы могли видеть образец использования функции.
Функция прилично сложная, поэтому давайте перейдем на это, чтобы убедиться, что мы понимаем, что происходит. У нас есть Действительно () Функция, которая принимает параметры поле и Видимое поле Отказ Это используется в контексте HTML-формы, поэтому два параметра являются элементами HTML. Мы видим переменную немедленно объявленную под названием Государство Отказ У этого есть Сохранено Собственность и A Требования имущество.
Одним из непосредственных проблем, которые вы можете заметить, это то, что Сохранено Недвижимость в Государство даже не используется. Вместо того, чтобы запутать вас, объяснив его оригинальные цели, давайте просто принять, что был план для его начального развития, который был заброшен, делая Сохранено Свойство артефакт старого дизайна (это никогда не было убрано).
Ключи в Требования Недвижимость в Государство Объект отображается на имена поля в форме ( Описание и Duedate находятся в нашей форме HTML). Требования Значения свойств, которые являются объектами, карта на разные валидации, которые мы хотим выполнить на поле. Например, если у нас есть …
// ...
requirements: {
Description: {
required: true,
maxlength: 150
},
// ...
}
… наша максимальная длина, если блок ловит его и возвращает ложь Если это не удается.
// max length
if (state.requirements[field.name].maxlength) {
if (field.value.length > state.requirements[field.name].maxlength) {
errorField.classList.add('inputBorderError');
return false;
}
}
Мы также можем увидеть, что функциональные ручки отображают ошибку, добавляя класс к элементу ( Errorfield.ClassList.add («Inputbobererror») ). Если Видимое поле Элемент предусмотрен, то есть то, на которую отображается ошибка, в противном случае она использует основной поле элемент.
Если поле проходит через все правила проверки, которые относятся к нему без возврата ложь функция в конечном итоге возвращает правда Таким образом, функция всегда возвращает логию.
Теперь, когда у нас есть базовое понимание того, как работает эта функция, давайте очистим ее.
Рефакторинг
Примечание: прежде чем мы продолжим, я приглашаю вас сделать попытку улучшить эту функцию самостоятельно. Не стесняйтесь поделиться своим решением в комментариях вместе с деталями, почему вы сделали то, что вы сделали, это может быть лучше, чем мой!
Первый Давайте начнем с чего-то легко. Как я уже говорил ранее, Сохранено Недвижимость в Государство больше не является частью решения, поэтому давайте удалим это.
function valid(field, visibleField) {
var state = {
// saved: true,
// ...
};
// ...
}
Второй Мне не нравится, что эта функция управляет отображением ошибок, когда проверка не удалась. Это «невидимый» побочный эффект, который делает эту функцию обманчивой, и что-то мы должны стараться избегать как можно больше. Никто не будет знать, что эта функция делает это, если они не читают содержимое функции, которые кому-то не нужно делать каждый раз, когда им это нужно. Функция называется Действительно , не ValidateandddisplayErrors Отказ Это также дополнительная ответственность, и мы хотим, чтобы наши функции были сосредоточены. Давайте удалим обработку ошибок в целом.
function valid(field) {
var state = {
requirements: {
Description: {
required: true,
maxlength: 150
},
DueDate: {
date: true
},
PriorityID: {},
TypeID: {}
}
};
if (!state.requirements[field.name]) {
return true;
}
// required
if (state.requirements[field.name].required) {
if (field.tagName.toLowerCase() == 'input' && field.value.length == 0) {
return false;
} else if (field.value === undefined || field.value === '') {
return false;
}
}
// max length
if (state.requirements[field.name].maxlength) {
if (field.value.length > state.requirements[field.name].maxlength) {
return false;
}
}
// date
if (state.requirements[field.name].date) {
if (!moment(field.value, ['MM/DD/YYYY', 'YYYY-M-D'], true).isValid()) {
return false;
}
}
return true;
}
Это позволило нам избавиться от нашего второго параметра, делая нашу функцию, которая намного проще.
Третий Пока мы снимаем обязанности, давайте удалим еще один. По какой-то причине эта функция жестко кодирует объект, который содержит правила проверки для одной конкретной формы с нашими Государство Переменная. Давайте удалим это и сделайте каждую функцию вызовов передавать правила проверки для этого элемента. К сожалению, это означает добавление второго параметра обратно.
function valid(field, validationRules) {
if (validationRules === undefined || validationRules === '')
return true;
// required
if (validationRules.required) {
if (field.tagName.toLowerCase() == 'input' && field.value.length == 0) {
return false;
} else if (field.value === undefined || field.value === '') {
return false;
}
}
// max length
if (validationRules.maxlength) {
if (field.value.length > validationRules.maxlength) {
return false;
}
}
// date
if (validationRules.date) {
if (!moment(field.value, ['MM/DD/YYYY', 'YYYY-M-D'], true).isValid()) {
return false;
}
}
return true;
}
Так что теперь наше использование выглядит так:
Четвертый Одна вещь, которая меня связывает меня сейчас, это функция зависит от Htmlelement Интерфейс Отказ Это не хорошо для тестирования, и это ненужная зависимость, потому что поле больше не используется для обработки ошибок. Мы боремся с разными типами тегов в некоторых случаях, чтобы в конечном итоге получить значение элемента, поэтому давайте просто передам стоимость прямо и избавимся от этого громоздкого бремени.
function valid(value, validationRules) {
if (
(typeof validationRules === 'object' && Object.keys(validationRules).length === 0)
|| validationRules === undefined
|| validationRules === ''
) {
return true;
}
// required
if (validationRules.required) {
if (!! value)
return false;
}
// max length
if (validationRules.maxlength) {
if (value.length > validationRules.maxlength)
return false;
}
// date
if (validationRules.date) {
if (!moment(value, ['MM/DD/YYYY', 'YYYY-M-D'], true).isValid())
return false;
}
return true;
}
Эта функция улучшилась резко от того, когда мы начали. Если вы остановились здесь, вы могли бы чувствовать себя довольно уверенно, доверяя ему достичь того, что ему нужно. Я собираюсь взять его немного дальше, хотя.
Пятый Эти эти отчеты блоки ощущаются примитивными. Я думаю, что мы можем сделать лучше. Им не хватает ясности и читабельности. Вместо этого я хочу сделать эти «валидаторы» в своих собственных функциях, чтобы мы хотим редактировать один или добавить к ним, нам нужно только изменять небольшую часть. Это позволяет нам покидать нашу основную функцию, которая выполняет только проверку.
Процесс мысли, который я описываю, получено из Сплошные принципы Отказ O в твердом теле – это Открытый принцип -Остен для расширения, закрыты для модификации. Это означает, что мы хотим легко расширить нашу функцию проверки, имея возможность добавлять валидаторы без изменения существующего кода. Это также S для Принцип одной ответственности Поскольку мы нарушаем нашу одну большую функцию в меньших неизменных методах, которые имеют только одну причину для изменения.
Я все еще хочу сохранить функцию самостоятельной; Посмотрите, можете ли вы следовать тому, что я собираюсь сделать. Я хочу сохранить свои методы валидатора в пределах действительной функции. Давайте потянем наши валидаторы в свои собственные методы в местном объекте валидаторы Отказ
function valid(value, validationRules) {
var validators = {
required: function(value, parameter) {
if (!! value)
return {rule:'required', message:'This field is required.'};
return false;
},
maxlength: function(value, parameter) {
if (value.length > parameter)
return {rule:'maxlength', message:'Maximum length is ' + parameter + ' characters.'};
return false;
},
date: function(value, parameter) {
if (!moment(value, parameter, true).isValid())
return {rule:'date', message:'Not a valid date format, must match ' + parameter + '.'};
return false;
}
};
// ...
}
Мы обновили Validators для каждого возврата объекта ошибки с помощью правила, с которыми не удалось, и сообщение по умолчанию, пользователь может захотеть отобразить. Так как мы больше не обрабатываем ошибки в доме, мы хотим передать максимальную информацию о самой информации, которую мы можем дать максимально гибкости для пользователя. Существует разница между функцией, занимающейся работой, которая имеет невидимые побочные эффекты и возврат данных, которые не выполняют свою работу самостоятельно.
Шестой Давайте переделаем логику, которая проверяет, действителен ли наша стоимость или не основана на правилах проверки.
function valid(value, validationRules) {
var validators = {
//...
};
// bug fix here
if (validationRules.required === undefined && !value)
return [];
var errors = [];
var result;
for (var rule in validationRules) {
result = validators[rule](value, validationRules[rule]);
if (result) errors.push(result);
}
return errors;
}
Теперь наша действительная функция возвращает массив вместо булева – он вернет пустой массив, если нет ошибок, или массив наших объектов ошибок, которые не выполняют проверку.
Пока переписывая эту часть, я нашел ошибку – если ВалидацияРулы Параметр не включает в себя Требуется собственность, то мы не должны беспокоить другие правила, когда ценность пустой. Я пометил исправления выше с комментарий «исправить ошибку здесь».
Для обработки наших правил мы просто петлю через свойства ВалидацияРулы Параметр и вызывают соответствующий валидатор. Если результат, который возвращается, оценивает true (потому что это объект при выборе проверки), то мы нажимаем его в массив ошибок.
Примечание. Я знаю, что отсутствие уловов для обработки потенциальных вопросов, таких как использование несуществующего валидатора в ВалидацияРулс , но я хочу держать пример простой для учебных целей.
Седьмой , вы можете думать «Эй, каждый раз, когда вы называете эту функцию, вы переопределении каждого метода валидатора!» Большой поймать, если бы ты сделал! Это неэффективно, чтобы спросить Действительно () Функция для определения валидаторы Объект со всеми его методами каждый раз, когда вызывается функция, поэтому я собираюсь повернуть Действительно В переменной и назначьте его немедленно вызванной анонимной функции, которая возвращает закрытие. Это держит валидаторы В местном объеме создает их только один раз и позволяет мне продолжать использовать Действительно так же.
var valid = (function() {
var validators = {
required: function(value, parameter) {
if (!! value)
return {rule:'required', message:'This field is required.'};
return false;
},
maxlength: function(value, parameter) {
if (value.length > parameter)
return {rule:'maxlength', message:'Maximum length is ' + parameter + ' characters.'};
return false;
},
date: function(value, parameter) {
if (!moment(value, parameter, true).isValid())
return {rule:'date', message:'Not a valid date format, must match ' + parameter + '.'};
return false;
}
};
return function(value, validationRules) {
if (validationRules.required === undefined && !value)
return [];
var errors = [];
var result;
for (var rule in validationRules) {
result = validators[rule](value, validationRules[rule]);
if (result) errors.push(result);
}
return errors;
};
})();
Это будет наш последний рефактором. Давайте посмотрим, как клиент использует нашу функцию сейчас.
Теперь мы проверяем длину массива, возвращаемого из вызова функции, чтобы определить, есть ли какие-либо ошибки. Если есть, мы можем получить элемент, который мы хотим отобразить обмен сообщениями и перечислять ошибки в нем и отображать его.
Рассмотрение
Возможно, вы думаете, что способ, которым мы взаимодействуем с этой функцией, стала более сложной, так как мы начали, и вы правы. Однако наша цель здесь состояла в том, чтобы исправить определенную функцию. Это включает в себя устранение других обязанностей, имело это, которое не должно было быть там. Прямо сейчас, это означает, что мы переехали на эту ответственность перед клиентом, но это не значит, что мы не можем написать другую функцию, которая использует наше Действительно Функция для обработки ошибок для нас.
Что мы можем сделать, это использовать наш новый Действительно Функция как строительный блок для функций более высокого уровня. Если мы хотим иметь функцию, которая намеренно имеет побочный эффект отображения ошибок, мы можем использовать наш Действительно функция внутри этого. Но мы держим валидационную часть, отделенную от других обязанностей, таких как отображение ошибок.
Мы также сократили зависимости в рамках функции, которая значительно расширяет удобство использования и гибкость этого. Например, удаление нашей зависимости от интерфейса HTMLELEATE позволяет использовать эту функцию для данных, возвращающихся из вызова AJAX, прежде чем отображать его, что было невозможно раньше.
В распылении валидаторов и предоставление каждой ответственности на одну ответственность, мы сделали функцию способом проще для работы наше будущее, а другие сначала знакомятся с ним. Если мы хотим добавить новый метод Validator, мы видим, какой вход и вывод остальных есть и скопируют его, или посмотрите, как наш основной цикл обработки работает с ними, чтобы узнать, как это реализовать (в языке OO Validators Скорее всего, реализует A Validator Интерфейс).
Когда мы создаем культуру высоких стандартов кодирования, где мы можем предположить, что функция с именем Действительно Проводит только проверку, мы увеличиваем доверие у разработчиков, работающих с кодом, потому что им не нужно читать содержимое каждой новой функции, напротив, чтобы убедиться, что невидимые побочные эффекты или другие странные взаимодействия. Мы освобождаем значительное количество времени и мощности мозга из-за этого. Тем меньше времени, потраченного на получение повторных потрачений с грязными, сложными функциями, тем больше времени проводилось на лучших вещах, таких как новые функции, изучение новых навыков и многое другое.