Skip to content

Latest commit

 

History

History
753 lines (537 loc) · 69.8 KB

CodeSmells.md

File metadata and controls

753 lines (537 loc) · 69.8 KB

Запахи кода и эвристические правила

Комментарии

С1: Неуместная информация

В комментариях неуместно размещать информацию, которую удобнее хранить в других источниках: в системах управления исходным кодом, в системах контроля версий и в других системах протоколирования. Например, история изменений только загромождает исходные файлы длинным историческим и малоинтересным текстом. Метаданные (авторы, дата последней модификации и т. д.) в общем случае также неуместны в комментариях. Комментарии должны быть зарезервированы для технической информации о коде и его архитектуре

C2: Устаревший комментарий

Комментарий, содержимое которого потеряло актуальность, считается устаревшим. Комментарии стареют довольно быстро. Не пишите комментарии, которые с течением времени устареют. Обнаружив устаревший комментарий, обновите его или избавьтесь от него как можно быстрее. Устаревшие комментарии часто «отрываются» от кода, который они когда-то описывали. Так в вашем коде появляются плавучие островки недостоверности и бесполезности

С3: Избыточный комментарий

Избыточным считается комментарий, описывающий то, что и так очевидно. Например:

i++; // Увеличение переменной i

Или другой пример — xml комментарий, который содержит не больше (а вернее, меньше) полезной информации, чем простая сигнатура функции:

/// <summary>
/// Некоторое действие
/// </summary>
/// <param name="count">Количество</param>
/// <returns></returns>
public void SomeAction(int count)
{

}

Комментарии должны говорить то, что не может сказать сам код.

C4: Плохо написанный комментарий

Если уж вы беретесь за написание комментария, напишите его хорошо. Не жалейте времени и позаботьтесь о том, чтобы это был лучший комментарий, который вы способны создать. Тщательно выбирайте слова. Следите за правильностью орфографии и пунктуации. Не пишите сумбурно. Не объясняйте очевидное. Будьте лаконичны.

C5: Закомментированный код

Фрагменты закомментированного кода выводят меня из себя. Кто знает, когда был написан этот код? Кто знает, есть от него какая-нибудь польза или нет? Однако никто не удаляет закомментированный код — все считают, что он понадобится кому-то другому. Этот код только попусту занимает место, «загнивая» и утрачивая актуальность с каждым днем. В нем вызываются несуществующие функции. В нем используются переменные, имена которых давно изменились. В нем соблюдаются устаревшие конвенции. Он загрязняет модуль, в котором он содержится, и отвлекает людей, которые пытаются его читать. Закомментированный код отвратителен! Увидев закомментированный код, удалите его! Не беспокойтесь, система управления исходным кодом его не забудет. Если кому-то этот код действительно понадобится, то он сможет вернуться к предыдущей версии. Не позволяйте закомментированному коду портить вам жизнь.

Функции

F1: Слишком много аргументов

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

F2: Выходные аргументы

Выходные аргументы противоестественны. Читатель кода ожидает, что аргументы используются для передачи входной, а не выходной информации. Если ваша функция должна изменять чье-либо состояние, пусть она изменяет состояние объекта, для которого она вызывалась

Исключением из этого правила могут послужить методы формата TrySomeAction, но и их использование сомнительно и не всегда оправданно, можно воспользоваться обёрткой Result или выбрасывать исключение при неудаче

F3: Флаги в аргументах

Логические аргументы явно указывают на то, что функция выполняет более одной операции. Они сильно запутывают код. Исключите их из своих программ

F4: Мёртвые функции

Если метод ни разу не вызывается в программе, то его следует удалить. Хранить «мертвый код» расточительно. Не бойтесь удалять мертвые функции. Не забудьте, что система управления исходным кодом позволит восстановить их в случае необходимости

F6: Инкапсулируйте условные конструкции

В булевской логике достаточно трудно разобраться и вне контекста команд if или while. Выделите в программе функции, объясняющие намерения условной конструкции. Например, команда

if (ShouldBeDeleted(timer))

выразительнее команды

if (timer.HasExpired() && !timer.IsRecurrent())

F7: Инкапсулируйте граничные условия

Отслеживать граничные условия нелегко. Разместите их обработку в одном месте. Не позволяйте им «растекаться» по всему коду. Не допускайте, чтобы в вашей программе кишели многочисленные +1 и –1. Возьмем простой пример

if(level + 1 < tags.length)
{
	parts = new Parse(body, tags, level + 1, offset + endTag);
	body = null;
}

Обратите внимание: level+1 здесь встречается дважды. Это граничное условие, которое следует инкапсулировать в переменной — например, с именем nextLevel:

int nextLevel = level + 1;
if(nextLevel < tags.length)
{
	parts = new Parse(body, tags, nextLevel, offset + endTag);
	body = null;
}

F8: Функции должны быть написаны на одном уровне абстракции

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

public String Render()
{
 StringBuffer html = new StringBuffer("<hr");
 if(size > 0)
	 html.append(" size=\"").append(size + 1).append("\"");
 html.append(">");
 return html.toString();
}

Разобраться в происходящем несложно. Функция конструирует тег HTML, который рисует на странице горизонтальную линию. Толщина линии задается переменной size

А теперь взгляните еще раз. В этом методе смешиваются минимум два уровня абстракции. Первый уровень — наличие толщины у горизонтальной линии. Второй уровень — синтаксис тега HR. Код позаимствован из модуля HruleWidget проекта FitNesse. Модуль распознает строку из четырех и более дефисов и преобразует ее в соответствующий тег HR. Чем больше дефисов, тем больше толщина.

Я переработал этот фрагмент кода так, как показано ниже. Обратите внимание: имя поля size изменено в соответствии с его истинным назначением (в нем хранится количество дополнительных дефисов)

public String Render()
{
	HtmlTag hr = new HtmlTag("hr");
	if (extraDashes > 0)
	hr.addAttribute("size", hrSize(extraDashes));
	return hr.html();
}

private String hrSize(int height)
{
 int hrSize = height + 1;
 return String.format("%d", hrSize);
}

Изменение разделяет два уровня абстракции. Функция render просто конструирует тег HR, ничего не зная о синтаксисе HTML этого тега. Модуль HtmlTag берет на себя все хлопоты с синтаксисом

Более того, при внесении этого изменения я обнаружил неприметную ошибку. Исходный код не закрывал тег HR косой чертой, как того требует стандарт XHTML (иначе говоря, он выдавал


вместо
), хотя модуль HtmlTag был давно приведен в соответствие со стандартом XHTML.

Разделение уровней абстракции — одна из самых важных и одновременно самых сложных в реализации функций рефакторинга

Имена

N1: Используйте содержательные имена

Не торопитесь с выбором имен. Позаботьтесь о том, чтобы имена были содержательными. Помните, что смысл может изменяться в ходе развития программного продукта. Почаще переосмысливайте уместность выбранных вами имен.

Не рассматривайте это как дополнительный «фактор комфортности». Имена в программных продуктах на 90% определяют удобочитаемость кода. Не жалейте времени на то, чтобы выбрать их осмысленно, и поддерживайте их актуальность. Имена слишком важны, чтобы относиться к ним легкомысленно.

N2: Выбирайте имена на подходящем уровне абстракции

Не используйте имена, передающие информацию о реализации. Имена должны отражать уровень абстракции, на котором работает класс или функция. Сделать это непросто — и снова потому, что люди слишком хорошо справляются со смешением разных уровней абстракции. При каждом просмотре кода вам с большой вероятностью попадется переменная, имя которой выбрано на слишком низком уровне. Воспользуйтесь случаем и измените его. Чтобы ваш код хорошо читался, вы должны серьезно относиться к его непрерывному совершенствованию

N3: По возможности используйте стандартную номенклатуру

Имена проще понять, если они основаны на существующих конвенциях или стандартных обозначениях. Например, при использовании паттерна ДЕКОРАТОР можно включить в имена декорирующих классов слово Decorator. Например, имя AutoHangupModemDecorator может быть присвоено классу, который дополняет класс Modem возможностью автоматического разрыва связи в конце сеанса. Паттерны составляют лишь одну разновидность стандартов. Например, в языке C# функции, преобразующие строковые представления в объекты, часто называются Parse. Лучше следовать подобным стандартным конвенциям, чем изобретать их заново.

Группы часто разрабатывают собственные стандартные системы имен для конкретного проекта. Широко используйте термины этой системы в своем коде. Чем больше вы используете имена, переопределенные специальным смыслом, относящимся к вашему конкретному проекту, тем проще читателю понять, о чем идет речь в вашем коде

N4: Недвусмысленные имена

Выбирайте имена, которые максимально недвусмысленно передают назначение функции или переменной. Рассмотрим пример

private string DoRename()
{
	if(_refactorReferences)
		RenameReferences();
	RenamePage();
	pathToRename.RemoveNameFromEnd();
	pathToRename.AddNameToEnd(newName);
	return PathParser.Render(pathToRename);
}

Имя функции получилось слишком общим и расплывчатым; оно ничего не говорит о том, что делает функция. Ситуацию усугубляет тот факт, что в функции с именем DoRename находится функция RenamePage! Что можно сказать о различиях между этими функциями по их именам? Ничего.

Функцию было бы правильнее назвать RenamePageAndOptionallyAllReferences. На первый взгляд имя кажется слишком длинным, но документирующая ценность перевешивает длину

N5: Используйте длинные имена для длинных областей видимости

Длина имени должна соответствовать длине его области видимости. Переменным с крошечной областью видимости можно присваивать очень короткие имена, но у переменных с большей областью видимости имена должны быть длинными. Если область видимости переменной составляет всего пять строк, то переменной можно присвоить имя i или j. Возьмем следующий фрагмент из старой стандартной игры «Bowling»:

private void RollMany(int n, int pins)
{
	for (int i = 0; i < n; i++)
		g.Roll(pins);
}

Смысл переменной i абсолютно очевиден. Какое-нибудь раздражающее имя вида rollCount только затемнило бы смысл этого тривиального кода. С другой стороны, смысл коротких имен переменных и функций рассеивается на длинных дистанциях. Таким образом, чем длиннее область видимости имени, тем более длинным и точным должно быть ее имя.

N6: Имена должны описывать побочные эффекты

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

public ObjectOutputStream GetOos()
{
	if (m_oos == null) {
		m_oos = new ObjectOutputStream(m_socket.getOutputStream());
	}
	return m_oos;
}

Функция не ограничивается простым получением m_oos; она создает объект m_oos, если он не был создан ранее. Таким образом, эту функцию было бы правильнее назвать GetOrCreateOos

N7: Имена функций должны описывать выполняемую операцию

Взгляните на следующий код:

Date newDate = date.Add(5);

Как вы думаете, что он делает — прибавляет пять дней к date? А может, пять недель или часов? Изменяется ли экземпляр date, или функция возвращает новое значение Date без изменения старого? По вызову невозможно понять, что делает эта функция

Если функция прибавляет пять дней с изменением date, то она должна называться AddDaysTo или IncreaseByDays. С другой стороны, если функция возвращаетe новую дату, смещенную на пять дней, но не изменяет исходного экземпляра Date, то она должна называться DaysLater или DaysSince

Если вам приходится обращаться к реализации (или документации), чтобы понять, что делает та или иная функция, постарайтесь найти более удачное имя или разбейте функциональность на меньшие функции с более понятными именами

Структура класса

S1: Несколько классов в одном файле

C# позволяет определять несколько классов в одном файле, но этого следует избегать. Это увеличивает время, затрачиваемое на поиск нужного файла, а так же усложняет чтение других классов, расположенных в том же файле.

В одном файле должен быть только один класс, и названия их должны совпадать чтобы у читателей не возникало сложностей с поиском и чтением кода.

Исключением может быть только служебный приватный внутренний класс, не использующийся нигде больше, но с этим нужно быть аккуратным

S2: Вертикальное разделение

Переменные и функции должны определяться вблизи от места их использования. Локальные переменные должны объявляться непосредственно перед первым использованием и должны обладать небольшой вертикальной областью видимости. Объявление локальной переменной не должно отдаляться от места ее использования на сотню строк

S3: Неуместные статические методы

Math.max(double a, double b) — нормальный статический метод. Он работает не с одним экземпляром; в самом деле, запись вида new Math().max(a,b) или даже a.max(b) выглядела бы довольно глупо. Все данные, используемые max, берутся из двух аргументов, а не из некоего объекта-«владельца». А главное, что метод Math.max почти наверняка не потребуется делать полиморфным.

Но иногда мы пишем статические функции, которые статическими быть не должны. Пример:

HourlyPayCalculator.CalculatePay(employee, overtimeRate)

Эта статическая функция тоже выглядит вполне разумно. Она не работает ни с каким конкретным объектом и получает все данные из своих аргументов. Однако нельзя исключать, что эту функцию потребуется сделать полиморфной. Возможно, в будущем потребуется реализовать несколько разных алгоритмов для вычисления почасовой оплаты — скажем, OvertimeHourlyPayCalculator и StraightTimeHourlyPayCalculator. В этом случае данная функция не может быть статической. Ее следует оформить как нестатическую функцию Employee.

В общем случае отдавайте предпочтение нестатическим методам перед статическими. Если сомневаетесь, сделайте функцию нестатической. Если вы твердо уверены, что функция должна быть статической, удостоверьтесь в том, что от нее не потребуется полиморфное поведение.

S4: Код на на неверном уровне абстракции

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

Например, константы, переменные и вспомогательные функции, относящиеся только к конкретной реализации, исключаются из базового класса. Базовый класс не должен ничего знать о них.

Правило также относится к исходным файлам, компонентам и модулям. Качественное проектирование требует, чтобы концепции разделялись на разных уровнях и размещались в разных контейнерах. Иногда такими контейнерами являются базовые и производные классы; в других случаях это могут быть исходные файлы, модули или компоненты. Но какое бы решение ни было выбрано в конкретном случае, разделение должно быть полным. Высокоуровневые и низкоуровневые концепции не должны смешиваться

public interface Stack 
{
	object Pop()
	void Push(object o);
	double PercentFull();
}

Функция PercentFull находится на неверном уровне абстракции. Существует много реализаций стека, в которых концепция заполнения выглядит разумно, однако другие реализации могут не знать, до какой степени заполнен стек. Следовательно, эта функция должна располагаться в производном интерфейсе — например, BoundedStack.

Возможно, вы думаете, что для неограниченного стека реализация может просто вернуть 0? Проблема в том, что абсолютно неограниченного стека не существует. Вам не удастся предотвратить исключение OutOfMemoryException, проверив условие

stack.PercemtFull() < 50.0

Если ваша реализация функции возвращает 0, то она попросту врет.

Суть в том, что ложь и фикции не способны компенсировать неверного размещения абстракций. Разделение абстракций — одна из самых сложных задач, решаемых разработчиками. Если выбор сделан неверно, не надейтесь, что вам удастся найти простое обходное решение.

S5: Слишком много информации

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

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

Скрывайте свои данные. Скрывайте вспомогательные функции. Скрывайте константы и временные переменные. Не создавайте классы с большим количеством методов или переменных экземпляров. Не создавайте большого количества защищенных переменных и функций в субклассах. Сосредоточьтесь на создании очень компактных, концентрированных интерфейсов. Сокращайте логические привязки за счет ограничения информации

S6: Функциональная зависимость

Для методов класса должны быть важны переменные и функции того класса, которому они принадлежат, а не переменные и функции других классов. Когда метод использует методы доступа другого объекта для манипуляций с его данными, то он завидует области видимости класса этого объекта. Он словно мечтает находиться в другом классе, чтобы иметь прямой доступ к переменным, с которыми он работает

public class HourlyPayCalculator {
 public Money CalculateWeeklyPay(HourlyEmployee e) {
	 int tenthRate = e.GetTenthRate().GetPennies();
	 int tenthsWorked = e.GetTenthsWorked();
	 int straightTime = Math.Min(400, tenthsWorked);
	 int overTime = Math.Max(0, tenthsWorked - straightTime);
	 int straightPay = straightTime * tenthRate;
	 int overtimePay = (int)Math.Round(overTime*tenthRate*1.5);
	 return new Money(straightPay + overtimePay);
 }
}

Метод CalculateWeeklyPay обращается к объекту HourlyEmployee за данными для обработки. Метод CalculateWeeklyPay завидует области видимости HourlyEmployee. Он «желает» получить доступ к внутренней реализации HourlyEmployee.

В общем случае от функциональной зависти следует избавиться, потому что она предоставляет доступ к «внутренностям» класса другому классу. Впрочем, иногда функциональная зависть оказывается неизбежным злом. Рассмотрим следующий пример

public class HourlyEmployeeReport {
 private HourlyEmployee _employee;
 public HourlyEmployeeReport(HourlyEmployee e) {
	 _employee = e;
 }
 string ReportHours() {
	 return String.format(
		 «Name: %s\tHours:%d.%1d\n»,
		 _employee.GetName(),
		 _employee.GetTenthsWorked()/10,
		 _employee.GetTenthsWorked()%10
		);
 }
}

Очевидно, метод ReportHours завидует классу HourlyEmployee. С другой стороны, мы не хотим, чтобы класс HourlyEmployee знал о формате отчета. Перемещение форматной строки в класс HourlyEmployee нарушает некоторые принципы объектно-ориентированного проектирования. Такое размещение привязывает HourlyEmployee к формату отчета и делает его уязвимым для изменений в этом формате

S7: Скрытые временные привязки

Временные привязки часто необходимы, но они не должны скрываться. Структура аргументов функций должна быть такой, чтобы последовательность вызова была абсолютно очевидной. Рассмотрим следующий пример

public class MoogDiver {
 Gradient gradient;
 List<Spline> splines;
 public void dive(String reason) {
	 saturateGradient();
	 reticulateSplines();
	 diveForMoog(reason);
 }
 ...
}

Порядок вызова трех функций важен. Сначала вызывается saturateGradient(), затем reticulateSplines() и только после этого diveForMoog(). К сожалению, код не обеспечивает принудительного соблюдения временной привязки. Ничто не мешает другому программисту вызвать reticulateSplines до saturateGradient, и все кончится исключением UnsaturatedGradientException.

Более правильное решение выглядит так:

public class MoogDiver {
 public void dive(String reason) {
	 Gradient gradient = saturateGradient();
	 List<Spline> splines = reticulateSplines(gradient);
	 diveForMoog(splines, reason);
 }
 ...
}

Временная привязка реализуется посредством создания «эстафеты». Каждая функция выдает результат, необходимый для работы следующей функции, и вызвать эти функции с нарушением порядка сколько-нибудь разумным способом уже не удастся.

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

S8: Поддерживайте согласованность данных

Типы нужно проектировать так, чтобы они содержали всю нужную инициализацию в конструкторе. Созданный экземпляр должен быть инициализирован конструктором, следует избегать такаих конструкций:

var group = new Group();
group.Name = "My group";

Нужно стараться избегать лишних публичных сеттеров и других методов, которые могут нарушить консистентность экземпляра.

public class Group
{
	public string Name { get; }

	public Group(string name)
	{
		Name = name;
	}
}

var group = new Group("My group");
group.Name = "My group"; // Will not compile

Структура модуля

M1: Неверное размещение

Одно из самых важных решений, принимаемых разработчиком, — выбор места для размещения кода. Например, где следует объявить константу PI? В классе Math? А может, ей место в классе Trigonometry? Или в классе Circle?

В игру вступает принцип наименьшего удивления. Код следует размещать там, где читатель ожидает его увидеть. Константа PI должна находиться там, где объявляются тригонометрические функции. Константа OVERTIME_RATE объявляется в классе HourlyPayCalculator

Иногда мы пытаемся «творчески» подойти к размещению функциональности. Мы размещаем ее в месте, удобном для нас, но это не всегда выглядит естественно для читателя кода. Предположим, потребовалось напечатать отчет с общим количеством отработанных часов. Мы можем просуммировать часы в коде, печатающем отчет, или же накапливать сумму в коде обработки учетных карточек рабочего времени

Чтобы принять решение, можно посмотреть на имена функций. Допустим, в модуле отчетов присутствует функция с именем getTotalHours, а в модуле обработки учетных карточек присутствует функция saveTimeCard. Какая из этих двух функций, если судить по имени, наводит на мысль о вычислении суммы?

Ответ очевиден.

Очевидно, по соображениям производительности сумму правильнее вычислять при обработке карточек, а не при печати отчета. Все верно, но этот факт должен быть отражен в именах функций. Например, в модуле обработки учетных карточек должна присутствовать функция computeRunningTotalOfHours

M2: Избегайте транзитивных обращений

В общем случае модуль не должен обладать слишком полной информацией о тех компонентах, с которыми он взаимодействует. Точнее, если A взаимодействует с B, а B взаимодействует с C, то модули, использующие A, не должны знать о C (то есть нежелательны конструкции вида a.getB().getC().doSomething();). Иногда это называется «законом Деметры». Прагматичные программисты используют термин «умеренный код»

В любом случае все сводится к тому, что модули должны обладать информацией только о тех модулях, с которыми они непосредственно взаимодействуют, а не располагать навигационной картой всей системы.

Если в нескольких модулях используется та или иная форма команды a.getB(). getC(), то в дальнейшем вам будет трудно изменить архитектуру системы, вставив между B и C промежуточный компонент Q. Придется найти каждое вхождение a.getB().getC() и преобразовать его в a.getB().getQ().getC(). Так образуются жесткие, закостеневшие архитектуры. Слишком многие модули располагают слишком подробной информацией о системе.

myCollaborator.doSomething()

M3: Базовые классы, завясящие от производных

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

Конечно, у этого правила имеются свои исключения. Иногда количество производных классов жестко фиксировано, а в базовом классе присутствует код для выбора между производными классами. Подобная ситуация часто встречается в реализациях конечных автоматов. Однако в этом случае между базовым и производными классами существует жесткая привязка, и они всегда размещаются вместе в одном файле dll. В общем случае нам хотелось бы иметь возможность размещения производных и базовых классов в разных файлах dll.

Размещение производных и базовых классов в разных файлах dll, при котором базовые файлы dll ничего не знают о содержимом производных файлов dll, позволяет организовать развертывание систем в формате дискретных, независимых компонентов. Если в такие компоненты будут внесены изменения, то они развертываются заново без необходимости повторного развертывания базовых компонентов. Такая архитектура значительно сокращает последствия от вносимых изменений и упрощает сопровождение систем в условиях реальной эксплуатации

M4: Искуственные привязки

То, что не зависит друг от друга, не должно объединяться искусственными привязками. Например, обобщенные перечисления не должны содержаться в более конкретных классах, потому что в этом случае информация о конкретном классе должна быть доступна в любой точке приложения, в которой используется перечисление. То же относится и к статическим функциям общего назначения, объявляемым в конкретных классах

В общем случае искусственной считается привязка между двумя модулями, не имеющая явной, непосредственной цели. Искусственная привязка возникает в результате размещения переменной, константы или функции во временно удобном, но неподходящем месте. Главные причины для появления искусственных привязок — лень и небрежность.

Не жалейте времени — разберитесь, где должно располагаться объявление той или иной функции, константы или переменной. Слишком часто мы размещаем их в удобном месте «под рукой», а потом оставляем там навсегда.

C#

CS1: Старайтесь избегать SQL-style LINQ

Первоначальный SQL-вид для LINQ сильно выделяется на фоне всего кода. Для разработчика, особенно не имеющего большого опыта работы с SQL, такой код сложнее читать и поддерживать. Fluent стиль позволяет явно продемонстрировать, что делает ваш Linq запрос.

CS2: Не используйте reflection для доступа к приватным полям или методам

Механизм рефлексии - мощный инструмент, позволяющий получить множество метаданных о типе. Если вы пытаетесь использовать его для обхода контракта класса, то нарушаете спроектированную архитектуру и создаёте крайне сильную зависимость на используемом классе. При этом зависимость будет не явной, так что при изменении класса, из которого мы вытаскиваем поля, написанный нами код с большой вероятностью перестанет работать. Это потратит много времени на поиск и устранение ошибки.

CS3: Минимизируйте количество downcast

Избегайте кастов там, где можно их не использовать. Программа должна стремиться к повышению типизации и увеличении количества мест, где происходят проверки во время компиляции. Если в методе приходится кастить аргументы, это значит, что нужно подумать над изменением сигнатуры метода. Возможно, от пользователя нужно требовать более конкретный тип.

Разное

G1: Очевидное поведение не реализовано

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

Day day = DayDate.Parse(String dayName);

Логично ожидать, что строка "Monday" будет преобразована в Day.MONDAY. Также можно ожидать, что будут поддерживаться стандартные сокращения дней недели, а регистр символов будет игнорироваться. Если очевидное поведение не реализовано, читатели и пользователи кода перестают полагаться на свою интуицию в отношении имен функций. Они теряют доверие к автору кода и им приходится разбираться во всех подробностях реализации

G2: Некорректное граничное поведение

Код должен работать правильно — вроде бы очевидное утверждение. Беда в том, что мы редко понимаем, насколько сложным бывает правильное поведение. Разработчики часто пишут функции, которые в их представлении работают, а затем доверяются своей интуиции вместо того, чтобы тщательно проверить работоспособность своего кода во всех граничных и особых ситуациях.

Усердие и терпение ничем не заменить. Каждая граничная ситуация, каждый необычный и особый случай способны нарушить работу элегантного и интуитивного алгоритма. Не полагайтесь на свою интуицию. Найдите каждое граничное условие и напишите для него тест

G3: Отключенные средства безопастности

Отключать средства безопасности рискованно. Ручное управление serialVersionUID бывает необходимо, но оно всегда сопряжено с риском. Иногда отключение некоторых (или всех!) предупреждений компилятора позволяет успешно построить программу, но при этом вы рискуете бесконечными отладочными сеансами. Не отключайте сбойные тесты, обещая себе, что вы заставите их проходить позднее, — это так же неразумно, как считать кредитную карту источником бесплатных денег

G4: Дублирование кода

Каждый раз, когда в программе встречается повторяющийся код, он указывает на упущенную возможность для абстракции. Возможно, дубликат мог бы стать функцией или даже отдельным классом. «Сворачивая» дублирование в подобные абстракции, вы расширяете лексикон языка программирования. Другие программисты могут воспользоваться созданными вами абстрактными концепциями. Повышение уровня абстракции ускоряет программирование и снижает вероятность ошибок.

Простейшая форма дублирования — куски одинакового кода. Программа выглядит так, словно у программиста дрожат руки, и он снова и снова вставляет один и тот же фрагмент. Такие дубликаты заменяются простыми методами. Менее тривиальная форма дублирования — цепочки switch/case или if/else, снова и снова встречающиеся в разных модулях и всегда проверяющие одинаковые наборы условий. Вместо них надлежит применять полиморфизм. Еще сложнее модули со сходными алгоритмами, но содержащие похожих строк кода. Однако дублирование присутствует и в этом случае.

Проблема решается применением паттернов ШАБЛОННЫЙ МЕТОД или СТРАТЕГИЯ. В сущности, большинство паттернов проектирования, появившихся за последние 15 лет, представляет собой хорошо известные способы борьбы с дублированием. Нормальные формы Кодда устраняют дублирование в схемах баз данных. Само объектно-ориентированное программирование может рассматриваться как стратегия модульной организации кода и устранения дубликатов.

G5: Мёртвый код

Мертвым кодом называется код, не выполняемый в ходе работы программы. Он содержится в теле команды if, проверяющей невозможное условие. Он содержится в секции catch для блока try, никогда не инициирующего исключения. Он содержится в маленьких вспомогательных методах, которые никогда не вызываются, или в никогда не встречающихся условиях switch/case.

Мертвый код плох тем, что спустя некоторое время он начинает «плохо пахнуть». Чем древнее код, тем сильнее и резче запах. Дело в том, что мертвый код не обновляется при изменении архитектуры. Он компилируется, но не соответствует более новым конвенциям и правилам. Он был написан в то время, когда система была другой. Обнаружив мертвый код, сделайте то, что положено делать в таких случаях: достойно похороните его. Удалите его из системы.

G6: Непоследовательность

Если некая операция выполняется определенным образом, то и все похожие операции должны выполняться так же. Это правило возвращает нас к «принципу наименьшего удивления». Ответственно подходите к выбору новых схем и обозначений, а если уж выбрали — продолжайте следовать им.

Если в функцию включена переменная response для хранения данных HttpServletResponse, будьте последовательны и используйте такое же имя переменной в других функциях, работающих с объектами HttpServletResponse. Если метод называется ProcessVerificationRequest, присваивайте похожие имена (например, ProcessDeletionRequest) методам, обрабатывающим другие запросы.

Последовательное соблюдение подобных схем и правил существенно упрощает чтение и модификацию кода.

G7: Непонятные намерения

Код должен быть как можно более выразительным. Слишком длинные выражения, венгерская запись, «волшебные числа» — все это скрывает намерения автора. Не жалейте времени на то, чтобы сделать намерения своего кода максимально прозрачными для читателей

G8: Избегайте отрицательных условий

Отрицательные условия немного сложнее для понимания, чем положительные. Таким образом, по возможности старайтесь формулировать положительные условия. Например, запись

if (buffer.shouldCompact())

предпочтительнее записи

if (!buffer.shouldNotCompact())

G9: Используйте пояснительные переменные

Один из самых эффективных способов улучшения удобочитаемости программы заключается в том, чтобы разбить обработку данных на промежуточные значения, хранящиеся в переменных с содержательными именами.

Matcher match = headerPattern.Matcher(line);
if(match.Find())
{
	String key = match.Group(1);
	String value = match.Group(2);
	headers.put(key.ToLowerCase(), value);
}

Простое использование пояснительных переменных четко объясняет, что первое совпадение содержит ключ (key), а второе — значение (value).

Перестараться в применении пояснительных переменных трудно. Как правило, чем больше пояснительных переменных, тем лучше. Поразительно, насколько очевидным иногда становится самый невразумительный модуль от простого разбиения обработки данных на промежуточные значения с удачно выбранными именами.

G10: Понимание алгоритма

Очень много странного кода пишется из-за того, что люди не утруждают себя пониманием алгоритмов. Они заставляют программу работать «грубой силой», набивая ее командами if и флагами, вместо того чтобы остановиться и подумать, что же в действительности происходит.

Программирование часто сопряжено с исследованиями. Вы думаете, что знаете подходящий алгоритм для решения задачи, но потом вам приходится возиться с ним, подправлять и затыкать щели, пока вы не заставите его «работать». А как вы определили, что он «работает»? Потому что алгоритм прошел все тесты, которые вы смогли придумать

В этом подходе нет ничего плохого. Более того, часто только так удается заставить функцию делать то, что она должна делать (по вашему мнению). Однако ограничиться «работой» в кавычках недостаточно.

Прежде чем откладывать в сторону готовую функцию, убедитесь в том, что вы понимаете, как она работает. Прохождения всех тестов недостаточно. Вы должны знать, что ваше решение правильно

Один из лучших способов достичь этого знания и понимания - разбить функцию на фрагменты настолько чистые и выразительные, что вам станет совершенно очевидно, как работает данная функция

G11: Используйте полиморфизм вместо if/else или switch/case

Команды switch чаще всего используются только потому, что они представляют очевидное решение методом «грубой силы», а не самое уместное решение для конкретной ситуации. Таким образом, это эвристическое правило напоминает нам о том, что до применения switch следует рассмотреть возможность применения полиморфизма

Так же ситуации, в которых состав функций менее стабилен, чем состав типов, встречаются относительно редко. Следовательно, к каждой конструкции switch следует относиться с подозрением

Можно придерживаться правила «ОДНОЙ КОМАНДЫ SWITCH»: для каждого типа выбора программа не должна содержать более одной команды switch. Множественные конструкции switch следует заменять полиморфными объектами.

G12: Заменяйте “волшебные числа” именованными константами

Например, число 86,400 следует скрыть в константе SECONDS_PER_DAY. Если в странице отчета выводится 55 строк, число 55 следует скрыть в константе LINES_PER_ PAGE

Некоторые числа так легко узнаются, что их не обязательно скрывать за именованными константами — при условии, что они используются в сочетании с предельно ясным кодом. Пример:

double milesWalked = feetWalked/5280.0;
int dailyPay = hourlyRate * 8;
double circumference = radius * Math.PI * 2;

Нужны ли константы FEET_PER_MILE, WORK_HOURS_PER_DAY и TWO в этих примерах? Разумеется, последний случай выглядит особенно абсурдно. В некоторых формулах константы попросту лучше воспринимаются в числовой записи. По поводу WORK_HOURS_PER_DAY можно спорить, потому что законы и нормативы могут изменяться. С другой стороны, формула с числом 8 читается настолько удобно, что мне просто не хочется нагружать читателя кода лишними 17 символами. А число 5280 — количество футов в миле — настолько хорошо известно и уникально, что читатель сразу узнает его, даже если оно будет располагаться вне какого-либо контекста.

Такие константы, как 3.141592653589793, тоже хорошо известны и легко узнаваемы. Однако вероятность ошибки слишком велика, чтобы оставлять их в числовой форме. Встречая значение 3.1415927535890793, вы сразу догадываетесь, что перед вами число π, и не проверяете его (а вы заметили ошибку в одной цифре?). Также мы не хотим, чтобы в программах использовались сокращения 3.14, 3.14159, 3.142 и т. д. К счастью, значение Math.PI уже определено за нас.

Термин «волшебное число» относится не только к числам. Он распространяется на все лексемы, значения которых не являются самодокументирующими. Пример:

assertEquals(7777, Employee.find("John Doe").employeeNumber());

В этом проверочном условии задействованы два «волшебных числа». Очевидно, первое — 7777, хотя его смысл далеко не так очевиден. Второе «волшебное число» — строка "John Doe". Ее смысл тоже выглядит весьма загадочно. Оказывается, "John Doe" — имя работника с табельным номером 7777 в тестовой базе данных, созданной нашей группой. Все участники группы знают, как подключаться к этой базе данных. В базе уже хранятся тестовые записи с заранее известными значениями и атрибутами. Также выясняется, что "John Doe" — единственный работник с почасовой оплатой в тестовой базе данных. Следовательно, эта проверка должна выглядеть так:

assertEquals(HOURLY_EMPLOYEE_ID, Employee.find(HOURLY_EMPLOYEE_NAME).employeeNumber());

G13: Валидируйте аргументы публичных методов перед использованием

Валидируйте аргументы перед их выполнением внутри методов, конструкторов:

  • Проверяйте на null
  • Проверяйте соответствует ли аргумент логике метода. Например, если у сущености есть заголовок, то мы ожидаем, что он не будет пустой строкой, а значит нужно в конструкторе это проверить.

G14: Используйте exception для повышения семантики

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

public Group(string name)
{
	var courseNumber = int.Parse(name.Substring(2, 1));
		// System.FormatException: Input string was not in a correct format
		// System.ArgumentOutOfRangeException: startIndex cannot be larger than length of string.
}

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

public Group(string name)
{
	if (name.Length < 5)
		throw new IsuException($"Invalid group name, name too short. Name : {name}");
	var courseNumberString = name.Substring(2, 1);
	if (!int.TryParse(courseNumberString, out int courseNumber))
		throw new IsuException($"Cannot parse course number from group {name}. Number: {courseNumberString }");
}

G15: Не создавайте большую вложенность

Не создавайте большую вложенность в тех местах, где можно без неё обойтись.

Инвертируйте условия, чтобы уменьшить вложенность. Вместо множества вложенных операторов выделяйте все граничные условия и обработйте их в самом начале метода для достижения большей читаемости кода. см. https://refactoring.guru/ru/replace-nested-conditional-with-guard-clauses