Pull to refresh

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

Reading time 6 min
Views 12K


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

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

В данной статье я хочу поделиться опытом преобразования весьма запутанного решения в приложение, разделенное на очевидные слои и покрытое юнит-тестами. Естественно, примеры, указанные в статье, в меру упрощены. На мой взгляд, их вполне достаточно для понимания основной концепции. Перед началом стоит сказать, что автор считает юнит-тестирование одним из наиболее весомых показателей качества кода. Также стоит уточнить, что решение написано на 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. Уменьшение моральных терзаний и очищение совести ;)

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

Articles

Information

Website
dnevnik.ru
Registered
Founded
Employees
101–200 employees
Location
Россия