Понятие хорошего или плохого дизайна является относительным. В то же время есть некоторые устоявшиеся нормы программирования, которые в большинстве случаев гарантируют ему эффективность, сопровождаемость, тестируемость. Например, в объектно-ориентированных языках это использование инкапсуляции, наследования, полиморфизма. Есть набор шаблонов проектирования, которые в ряде случаев дают положительный эффект на дизайн приложения (а иногда и отрицательный, все зависит от ситуации). С другой стороны, есть противоположные нормы, следование которым иногда приводит к дизайну, который можно назвать проблемным. Такой дизайн как правило обладает следующими признаками (каким-то одним или несколькими одновременно):
- Жесткость (трудно менять код, так как простое изменение затрагивает много мест);
- Неподвижность (сложно разделить код на модули, которые можно использовать в других программах);
- Вязкость (разрабатывать и/или тестировать код довольно тяжело);
- Ненужная Сложность (в коде есть неиспользуемый функционал);
- Ненужная Повторяемость (Copy/Paste);
- Плохая Читабельность (трудно понять что код делает, трудно его поддерживать);
- Хрупкость (легко поломать функционал даже небольшими изменениями);
Их нужно уметь понимать и различать для того, чтобы избежать проблемного дизайна либо предвидеть возможные последствия его использования. Эти признаки описаны в книге Роберта Мартина «Agile Principles, Patterns, And Practices in C#». Однако в ней, как и в некоторых других обзорных статьях по этой теме (которых, кстати, не очень много), приведено довольно краткое их описание и как правило отсутствуют примеры кода.
Попробуем исправить этот недостаток, более или менее подробно остановившись на каждом из этих признаков.
Жесткость
Как уже было обозначено выше, жесткий код бывает сложно менять, даже по мелочам. Это может и не быть проблемой в случае, если код меняется нечасто либо не меняется вообще. Таким образом, код получается вовсе даже неплохим. Но если он периодически требует изменений и делать это становится трудно, он становится проблемным. Даже если он работает.
Одним из распространенных случаев жесткости является явное указание типов классов вместо использования абстракций (интерфейсов, базовых классов и т.п.). Ниже приведен пример кода.
class A
{
B _b;
public A()
{
_b = new B();
}
public void Foo()
{
// Do some custom logic.
_b.DoSomething();
// Do some custom logic.
}
}
class B
{
public void DoSomething()
{
// Do something
}
}
Здесь класс A жестко зависит от класса B. То есть если в будущем будет принято решение использовать вместо B другой класс, это потребует изменение класса A и, следовательно, его повторного тестирования. Кроме того, если от B жестко зависит множество других классов, ситуация может многократно осложниться.
Выходом из ситуации может быть введение абстракции, то есть некоего интерфейса IComponent и внедрение такой зависимости через конструктор (либо как-то еще) класса A. В этом случае класс A перестает зависеть от конкретного класса B, а зависит лишь от интерфейса IComponent. Класс B, в свою очередь, должен реализовывать интерфейс IComponent.
interface IComponent
{
void DoSomething();
}
class A
{
IComponent _component;
public A(IComponent component)
{
_component = component;
}
void Foo()
{
// Do some custom logic.
_component.DoSomething();
// Do some custom logic.
}
}
class B : IComponent
{
void DoSomething()
{
// Do something
}
}
Давайте приведем более конкретный пример. Пусть есть некоторый набор классов, которые логируют информацию — ProductManager и Consumer. Их задача сохранить некоторый продукт в базе данных и заказать его соответственно. Оба класса логируют релевантные события. Пусть сначала логирование осуществлялось в файл. Для этого использовался класс FileLogger. При этом классы располагались в разных модулях (сборках).
// Module 1 (Client)
static void Main()
{
var product = new Product("milk");
var productManager = new ProductManager();
productManager.AddProduct(product);
var consumer = new Consumer();
consumer.PurchaseProduct(product.Name);
}
// Module 2 (Business logic)
public class ProductManager
{
private readonly FileLogger _logger = new FileLogger();
public void AddProduct(Product product)
{
// Add the product to the database.
_logger.Log("The product is added.");
}
}
public class Consumer
{
private readonly FileLogger _logger = new FileLogger();
public void PurchaseProduct(string product)
{
// Purchase the product.
_logger.Log("The product is purchased.");
}
}
public class Product
{
public string Name { get; private set; }
public Product(string name)
{
Name = name;
}
}
// Module 3 (Logger implementation)
public class FileLogger
{
const string FileName = "log.txt";
public void Log(string message)
{
// Write the message to the file.
}
}
Если в начале было достаточно только файла, а затем возникает необходимость логировать и в другие хранилища, такие как база данных или облачный сервис сбора и хранения информации, то потребуется изменение всех классов в модуле бизнес логики (Module 2), использующих FileLogger. В конечном итоге это может оказаться сложным. Для решения этой проблемы можно ввести абстрактный интерфейс для работы с логгером, как показано ниже.
// Module 1 (Client)
static void Main()
{
var logger = new FileLogger();
var product = new Product("milk");
var productManager = new ProductManager(logger);
productManager.AddProduct(product);
var consumer = new Consumer(logger);
consumer.PurchaseProduct(product.Name);
}
// Module 2 (Business logic)
class ProductManager
{
private readonly ILogger _logger;
public ProductManager(ILogger logger)
{
_logger = logger;
}
public void AddProduct(Product product)
{
// Add the product to the database.
_logger.Log("The product is added.");
}
}
public class Consumer
{
private readonly ILogger _logger;
public Consumer(ILogger logger)
{
_logger = logger;
}
public void PurchaseProduct(string product)
{
// Purchase the product.
_logger.Log("The product is purchased.");
}
}
public class Product
{
public string Name { get; private set; }
public Product(string name)
{
Name = name;
}
}
// Module 3 (interfaces)
public interface ILogger
{
void Log(string message);
}
// Module 4 (Logger implementation)
public class FileLogger : ILogger
{
const string FileName = "log.txt";
public virtual void Log(string message)
{
// Write the message to the file.
}
}
В этом случае при смене типа логгера достаточно поменять код клиента (Main), который инициализирует логгер и подкладывает его в конструктор ProductManager и Consumer. Таким образом мы закрыли классы бизнес логики от модификации в отношении типа логгера, что и требовалось сделать.
Кроме прямых ссылок на используемые классы, жесткость может проявляться любыми другими вариантами, приводящими к сложностям при изменении кода. Таковых может быть бесконечное множество, но попробуем привести еще один пример. Пусть есть некий код, который выводит на консоль площади геометрических фигур.
static void Main()
{
var rectangle = new Rectangle() { W = 3, H = 5 };
var circle = new Circle() { R = 7 };
var shapes = new Shape[] { rectangle, circle };
ShapeHelper.ReportShapesSize(shapes);
}
class ShapeHelper
{
private static double GetShapeArea(Shape shape)
{
if (shape is Rectangle)
{
return ((Rectangle)shape).W * ((Rectangle)shape).H;
}
if (shape is Circle)
{
return 2 * Math.PI * ((Circle)shape).R * ((Circle)shape).R;
}
throw new InvalidOperationException("Not supported shape");
}
public static void ReportShapesSize(Shape[] shapes)
{
foreach(Shape shape in shapes)
{
if (shape is Rectangle)
{
double area = GetShapeArea(shape);
Console.WriteLine($"Rectangle's area is {area}");
}
if (shape is Circle)
{
double area = GetShapeArea(shape);
Console.WriteLine($"Circle's area is {area}");
}
}
}
}
public class Shape
{ }
public class Rectangle : Shape
{
public double W { get; set; }
public double H { get; set; }
}
public class Circle : Shape
{
public double R { get; set; }
}
Из этого кода видно, что при добавлении новой фигуры придется менять методы класса ShapeHelper. Одним из вариантов может быть перенесение алгоритма отрисовки в сами классы геометрических фигур (Rectangle и Circle), как показано ниже. Этим мы изолируем релевантную логику в соответствующих классах, сузив тем самым ответственность класса ShapeHelper до вывода информации на консоль.
static void Main()
{
var rectangle = new Rectangle() { W = 3, H = 5 };
var circle = new Circle() { R = 7 };
var shapes = new Shape[]() { rectangle, circle };
ShapeHelper.ReportShapesSize(shapes);
}
class ShapeHelper
{
public static void ReportShapesSize(Shape[] shapes)
{
foreach(Shape shape in shapes)
{
shape.Report();
}
}
}
public abstract class Shape
{
public abstract void Report();
}
public class Rectangle : Shape
{
public double W { get; set; }
public double H { get; set; }
public override void Report()
{
double area = W * H;
Console.WriteLine($"Rectangle's area is {area}");
}
}
public class Circle : Shape
{
public double R { get; set; }
public override void Report()
{
double area = 2 * Math.PI * R * R;
Console.WriteLine($"Circle's area is {area}");
}
}
Как результат, мы фактически закрыли класс ShapeHelper для изменений по добавлению новых типов фигур за счет использования наследования и полиморфизма.
Неподвижность
Неподвижность проявляется в сложности разделения кода на переиспользуемые модули. В результате проект может потерять способность эволюционировать и как результат перестанет быть конкурентноспособным.
В качестве примера рассмотрим десктопную программу, весь код которой реализован в исполняемом файле приложения (exe) и был спроектирован таким образом, что вынесение бизнес логики в отдельные модули либо классы не предусмотрено. Затем, какое-то время спустя перед разработчиком программы возникли следующие бизнес требования:
- Изменить пользовательский интерфейс, сделав его Web приложением;
- Опубликовать функционал программы в виде набора Web сервисов, доступных сторонним клиентам для использования в их собственных приложениях.
В этом случае данные требования трудновыполнимы, поскольку весь код программы зашит в исполняемом модуле.
На рисунке ниже показан пример неподвижного дизайна в противовес тому, который этим признаком не страдает. Они разделены пунктирной линией. Как видно из рисунка, распределение кода по повторно используемым модулям (Logic), а также публикация функционала на уровне Web сервисов позволяет использовать его в различных клиентских приложениях (App), что является несомненным плюсом.
Неподвижность также можно назвать монолитностью дизайна. Его как каменную глыбу трудно раздробить на более мелкие и полезные куски кода. Как избежать этой проблемы? На этапе проектирования лучше подумать, насколько вероятно использование того или иного функционала в других системах. Потенциально переиспользуемый код лучше сразу поместить в отдельные модули и классы.
Вязкость
Вязкость можно подразделить на два типа:
- Вязкость разработки;
- Вязкость окружения.
Вязкость разработки проявляется в относительной трудности следования выбранному дизайну приложения. Это может происходить оттого, что к программисту предъявляется слишком много требований, которые необходимо исполнять, в то время как есть намного более удобный путь разработки.
Вязкость окружения может проявляться в неэффективности процесса сборки, развертывания и тестирования приложения.
В качестве простого примера, где переплетаются оба типа вязкости, можно привести работу с константами, которые требуется (By Design) помещать в отдельный модуль (Module 1) для использования другими компонентами (Module 2 и Module 3).
// Module 1 (Constants)
static class Constants
{
public const decimal MaxSalary = 100M;
public const int MaxNumberOfProducts = 100;
}
// Finance Module
#using Module1
static class FinanceHelper
{
public static bool ApproveSalary(decimal salary)
{
return salary <= Constants.MaxSalary;
}
}
// Marketing Module
#using Module1
class ProductManager
{
public void MakeOrder()
{
int productsNumber = 0;
while(productsNumber++ <= Constants.MaxNumberOfProducts)
{
// Purchase some product
}
}
}
Данный дизайн может оказаться вязким. Если по какой-то причине процесс сборки модуля констант занимает значительное время, разработчикам будет сложно ожидать его окончания (Вязкость окружения). Кроме того, можно обратить внимание на то, что в модуле констант содержатся достаточно разнородные сущности, относящиеся к принципиально разным частям бизнес логики (финансовый и маркетинговый модули). То есть модуль констант может меняться довольно часто по независимым друг от друга причинам, что может привести к дополнительным проблемам в виде процесса синхронизации изменений (Вязкость разработки).
Все это замедляет процесс разработки и может напрягать программистов. Вариантами менее вязкого дизайна было бы создание отдельных модулей констант — по одному на соответствующий модуль бизнес логики, либо перенос констант в нужное место без выделения под них отдельного модуля.
Еще одним примером Вязкости Окружения можно считать разработку и тестирование приложения на удаленной виртуальной машине клиента. Иногда такой рабочий процесс становится невыносимым из-за медленного интернет соединения, в связи с чем разработчик может систематически пренебрегать интеграционным тестированием написанного кода, что в итоге может привести к проблемам на клиентской стороне при использовании этого функционала (Bugs).
Ненужная Сложность
В этом случае дизайн имеет неиспользуемый в данный момент функционал (так сказать на будущее, а вдруг пригодится). Этот факт может затруднять поддержку и сопровождение программы, а также увеличивать время разработки и тестирования. Например, рассмотрим программу, которая требует чтения некоторых данных из базы. Для этого был написан специальный компонент DataManager, который используется в другом компоненте.
class DataManager
{
object[] GetData()
{
// Retrieve and return data
}
}
Если разработчик добавит в DataManager новый метод для записи данных в базу (WriteData), который на данный момент не используется, но с очень малой вероятностью может пригодиться в будущем, это будет проявлением признака Ненужная Сложность. Зачем тратить время на написания потенциально неиспользуемого кода? QA иногда обязаны тестировать такой код, потому что он фактически публикуется и становится доступным для использования сторонними клиентами. Это тоже отъедает время релиза. Закладывать функционал на будущее стоит лишь при условии того, что возможная выгода от его наличия превысит затраты на его разработку и тестирование.
Ненужная Повторяемость
Наверное большинство разработчиков сталкивалиcь или столкнуться с этим признаком, который заключается в множественном копировании либо одной и той же логики, либо некоторого участка кода целиком. Основной угрозой при этом является уязвимость такого кода при внесении изменений — поправив что-то в одном месте, можно забыть сделать это в другом. Кроме того, требуется больше времени при внесении изменений по сравнению с ситуацией, когда код лишен этого признака.
Ненужная Повторяемость может быть как следствием нерадивости разработчиков, так и Жесткости/Хрупкости дизайна, когда не повторять код гораздо труднее и рискованнее, чем делать это. Однако в любом случае нельзя попускать повторяемость и необходимо постоянно улучшать код, перенося повторно используемые участки в общие методы и классы.
Плохая Читабельность
Этот признак проявляется в том, что код трудно читать и понимать что он делает (и почему он делает именно так). Причинами плохой читабельности может быть несоблюдение требований к оформлению кода (синтаксис, именование переменных, классов и т.д.), запутанная логика реализации и другое.
Ниже приведен пример трудночитаемого кода, который реализует метод, оперирующий булевой переменной.
void Process_true_false(string trueorfalsevalue)
{
if (trueorfalsevalue.ToString().Length == 4)
{
// That means trueorfalsevalue is probably "true". Do something here.
}
else if (trueorfalsevalue.ToString().Length == 5)
{
// That means trueorfalsevalue is probably "false". Do something here.
}
else
{
throw new Exception("not true of false. that's not nice. return.")
}
}
Здесь можно выделить сразу несколько проблем. Во-первых, названия методов и переменных не подчиняются общепринятым конвенциям. Во-вторых, реализация метода не самая лучшая.
Возможно стоит сразу принимать булево значение, а не строку. Но даже если принимать строку, ее лучше сконвертировать в булеву величину в начале метода, а не пользоваться методом определения длины строки. В третьих, текст исключения (that's not nice) не соответствуют официально деловому стилю. Читая такие тексты, может сложиться ощущение, что код написан непрофессионалом (может быть спорный момент, но все же). Метод можно было бы переписать так (при условии, что он будет принимать булево значение, а не строку):
public void Process(bool value)
{
if (value)
{
// Do something.
}
else
{
// Do something.
}
}
Вот другой вариант рефакторинга (если принимать строку все же необходимо).
public void Process(string value)
{
bool bValue = false;
if (!bool.TryParse(value, out bValue))
{
throw new ArgumentException($"The {value} is not boolean");
}
if (bValue)
{
// Do something.
}
else
{
// Do something.
}
}
Над трудно читабельным кодом следует проводить рефакторинг, если он начинает приносить проблемы. Например в случае, если его сопровождение и размножение приводит к возникновению большого количества багов.
Хрупкость
Хрупкость программы означает простоту ее поломки в случае внесения изменений. Поломки бывают двух типов — ошибки компиляции и ошибки времени выполнения. Первые могут являться оборотной стороной Жесткости, вторые наиболее опасны, так как часто находятся уже на стороне клиента. Вот они то и являются показателем Хрупкости.
Показатель, бесспорно, относительный. Кто-то правит код очень внимательно и вероятность поломки низкая. Кто-то наоборот — в спешке и невнимательно. Но все же при одних и тех же исполнителях разный код может порождать разное количество ошибок. Наверное можно говорить о том, что чем неразборчивее он, чем сложнее, чем больше полагается на время выполнения программы нежели на этап компиляции, тем более он хрупок.
При этом часто ломается функционал, который даже не собирались менять. Он может пострадать из-за высокой связности логики различных компонентов между собой.
Рассмотрим конкретный пример. Здесь логика авторизации пользователя с некоторой ролью (определяется параметром roleId) по доступу к некоторому ресурсу (resourceUri) вынесена в статический метод.
static void Main()
{
if (Helper.Authorize(1, "/pictures"))
{
Console.WriteLine("Authorized");
}
}
class Helper
{
public static bool Authorize(int roleId, string resourceUri)
{
if (roleId == 1 || roleId == 10)
{
if (resourceUri == "/pictures")
{
return true;
}
}
if (roleId == 1 && roleId == 2 && resourceUri == "/admin")
{
return true;
}
return false;
}
}
Можно заметить, что логика довольно «волосатая». Очевидно, что при добавлении новых ролей и ресурсов ее будет несложно поломать. В результате, некоторая роль может получить либо потерять доступ к некоторому ресурсу. Уменьшить хрупкость помогло бы создание класса Resource, который внутри себя хранил бы идентификатор ресурса и список поддерживаемых ролей, как показано ниже.
static void Main()
{
var picturesResource = new Resource() { Uri = "/pictures" };
picturesResource.AddRole(1);
if (picturesResource.IsAvailable(1))
{
Console.WriteLine("Authorized");
}
}
class Resource
{
private List<int> _roles = new List<int>();
public string Uri { get; set; }
public void AddRole(int roleId)
{
_roles.Add(roleId);
}
public void RemoveRole(int roleId)
{
_roles.Remove(roleId);
}
public bool IsAvailable(int roleId)
{
return _roles.Contains(roleId);
}
}
В этом случае для добавления новых ресурсов и ролей вообще не требуется модификации кода логики авторизации, то есть ломать по сути остается нечего.
Что может помочь отловить ошибки времени выполнения? Ручное, автоматическое и юнит тестирование. Чем лучше организован процесс тестирования, тем больше защиты от того, что хрупкий код проявит себя на стороне клиента.
Часто хрупкость является оборотной стороной других признаков плохого дизайна, таких как Жесткость, Плохая Читабельность, Ненужная повторяемость.
Заключение
Мы попытались выделить и описать основные признаки проблемного дизайна. Некоторые их них взаимосвязаны. Нужно четко понимать, что проблемность дизайна не всегда неизбежно приводит к трудностям, она лишь указывает на вероятность их наступления. Чем меньше прослеживаются эти признаки, тем ниже эта вероятность.
Автор: Сергей Золотарев
