Рефакторинг с бубном, или как мы халка усмиряли

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

Первое решение, которое приходит в голову – это все переписать, держа подмышкой томики Фаулера и ГОФА. Есть но: навряд ли такое предложение воспримут всерьез. Второе решение, уже более приемлемое – попытаться маленькими шажками унизить Александра Македонского, и все-таки распутать Гордиев Узел.

В данной статье я хочу поделиться опытом преобразования весьма запутанного решения в приложение, разделенное на очевидные слои и покрытое юнит-тестами. Естественно, примеры, указанные в статье, в меру упрощены. На мой взгляд, их вполне достаточно для понимания основной концепции. Перед началом стоит сказать, что автор считает юнит-тестирование одним из наиболее весомых показателей качества кода. Также стоит уточнить, что решение написано на ASP.NET с использованием Webforms.

Итак, давайте начнем наши пляски:

Фигура первая – запутанная.

Самая первая проблема, с которой пришлось столкнуться – это смешение кода, ответственного за формирование html, с кодом DAL-а. То есть, вот такая писанина, находящаяся внутри UserControl-а, встречалась довольно часто:

StringBuilder content = new StringBuilder(); var persons = Database.Persons.GetListBySchool(school); foreach(var person in persons) {    content.AppendFormat(@”<p>{0}</p>”, person.Name); } 

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

public interface ICommand {    void Execute(); } 

public abstract class Command<T> : ICommand {    public T Result { get; protected set; }    public abstract void Execute(); } 

public interface ICommandExecutor {    T Run<T>(Command<T> command); } 

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

public class GetListBySchool: Command<List<DbPerson>> {    public DbSchool School { get; set; }      public override void Execute()    {       this.Result = Database.Persons.GetListBySchool(this.School);    } } 

А вот и реализация ICommandExecutor:

public T Run<T>(Command<T> command) {    command.Execute();    return command.Result; } 

При использовании данного решения, код внутри юзер-контролов превратился в:

private readonly ICommandExecutor executor;   public SchoolView(ICommandExecutor executor) {    if (executor == null) { throw new ArgumentNullException("executor"); }    this.executor = executor; }   public string GetPersonsHtmlList() {    StringBuilder content = new StringBuilder();    var persons = this.executor.Run(new GetListBySchool { School = school });    foreach(var person in persons)    {      content.AppendFormat(@”<p>{0}</p>”, person.Name);    } 

Вроде бы все хорошо, но сразу же после замещения всех прямых обращений к базе данных на команды стало ясно, что полученный обновленный код также невозможно протестировать. И причиной этому объекты, возвращаемые командой. Класс DbPerson содержал в себе много логики и свойства, не имеющие паблик сеттера. Первые несколько свойств были успешно помечены, как virtual, и замоканы. Но третье свойство имело тип, имеющий только private конструктор – это и стало камнем преткновения. В этот момент кто-то стукнул по столу кулаком и сказал, что, раз решили делать без костылей, то и нужно придерживаться такого направления. Свойства, помеченные virtual, были приведены в исходное положение, и мозг заработал в поисках другого варианта. На самом деле, решение изначально лежало на поверхности, просто ни у кого не хватало смелости его озвучить, ибо оно влекло за собой очень много нудной работы. А решение следующее: DTO. Это значит, что внутри UserControl-ов мы используем объекты, являющиеся копией объектов DAL, но без какой-либо программной логики, с публичными геттерами и сеттерами. Чтобы данное решение стало возможным, мы создали конвертеры. Для удобства были использованы extension-методы.

Команда стала выглядеть так:

public class GetListBySchool: Command<List<Person>> {    public long SchoolId { get; set; }      public override void Execute()    {       var dbSchool = DataBase.Instance.GetSchoolById(this.SchoolId);       this.Result = dbSchool.Network.Memberships          .GetListBySchool(this.School)          .Select(person => person.Convert())          .ToList();    } } 

Теперь наш метод GetPersonsHtmlList стал полностью тестируем, однако, появилась другая проблема: внутри UserControl-а подобный метод не единственный, их много. И если раньше объект DbSchool загружался только один раз, то теперь он загружается в каждой отдельной команде, а это не есть комильфо. Для решения данной проблемы к нам приходит IContextManager с методом

T GetFunctionResult<T>(Func<T> func) where T : class;  

С имплементацией мы особо не заморачивались, использовав HttpContext.Current. То есть, если метод вызывается первый раз, то объект кидается в контекст. Если метод вызывается второй, или более раз, то объект берется из контекста. Имплементация IContextManager задается в конструкторе CommandExecutor-а, и далее передается во все команды (у базового класса команды появилось новое свойство).

public T Run<T>(Command<T> command) {    command.Context = this.context;    command.Execute();    return command.Result; } 

public class GetListBySchool: Command<List<Person>> {    public long SchoolId { get; set; }      public override void Execute()    {       var dbSchool = this.Context.GetFunctionResult(() => DataBase.Instance.GetSchoolById(this.SchoolId));       this.Result = dbSchool.Network.Memberships          .GetListBySchool(this.School)          .Select(person => person.Convert())          .ToList();    } } 

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

Фигура вторая – очевидная.

Следующей задачей стало избавление от привязки к коллекции Request.Params. Но здесь ни у кого вопросов не возникло, и решение стало следующим:

public interface IParametersReader {     string GetRequestValue(string key); } 

public class RequestParametersReader : IParametersReader {     private readonly HttpContextBase context;       public RequestParametersReader(HttpContextBase context)     {         if (context == null) { throw new ArgumentNullException("context"); }         this.context = context;     }       public string GetRequestValue(string key)     {         return this.context.Request[key];     } } 

Фигура третья – воздушная.

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

void Bind(DbSchool school, DbPerson person) {      this.school = school;      this.person = person; } 

Затем метод вызывается из родительского UserControl. Вроде бы, все замечательно, и такое решение имеет право на существование. Однако, не стоит забывать: когда вложенность контролов – более двух уровней, значительно возрастает возможность ошибиться или забыть задать нужное значение.В качестве решения мы использовали довольно интересную концепцию Dependency Outjection. Её основная идея – получение данных не из заранее заданного источника, а из общей коллекции объектов, пополнить которую может любой. Для этого были созданы два класса:

public class InAttribute : Attribute {    public string Name { get; set; } } 

и

public class OutAttribute : Attribute {     public string Name { get; set; } } 

Те свойства, которые нужно повесить в воздухе, помечаются атрибутом Out. Те свойства, которые надо из этого воздуха заполнить, по аналогии помечаются атрибутом In.Дальше – лишь дело техники. Мы создали класс, наследуемый от System.Web.UI.UserControlИ добавили в него такой код:

protected override void OnLoad(EventArgs e) {      //Чтение данных из контекста      var inProperties = this.GetType()           .GetProperties()           .Where(prop => prop.GetCustomAttributes(typeof(InAttribute), false).Any());            foreach (var property in inProperties)            {                 var inAttribute =                    property.GetCustomAttributes(typeof(InAttribute), false).First() as InAttribute;                 if (inAttribute == null) { continue; }                   property.SetValue(                      this,                       this.context.Get(string.IsNullOrWhiteSpace(inAttribute.Name) ? property.Name : inAttribute.Name),                        null);             }         base.OnLoad(e);        //Запись данных в контекст      var outProperties = this.GetType()           .GetProperties()           .Where(prop => prop.GetCustomAttributes(typeof(OutAttribute), false).Any());            foreach (var property in outProperties)            {               var outAttribute = property.GetCustomAttributes(                   typeof(OutAttribute), false).First() as OutAttribute;               if (outAttribute == null) { continue; }                 this.context.Add(string.IsNullOrWhiteSpace(outAttribute.Name) ? property.Name : outAttribute.Name, property.GetValue(this, null));            }  } 

Эпилог:

Таким стал первый этап рефакторинга нашего зеленого монстра программного продукта. Итогом нашей работы стали:

1. Возможность полного покрытия UserControl-ов юнит-тестами.
2. Более четкое отделение слоя UI от всего остального.
3. Избавление от жесткой привязки между вложенными UserContol-ами.
4. Улучшение читаемости кода.
5. Уменьшение моральных терзаний и очищение совести 😉

Автор статьи: Виталий Лебедев, ведущий разработчик Дневник.ру