Оверинжиниринг головного мозга

Попалась мне простая развлекательная задача: собрать данные о температуре воды и воздуха с пары HTML страниц и выдать результат, в JSON из API. Задача тривиальная, решается кодом строк в 40 (или около того) с комментариями. Конечно если писать руководствуясь принципом Quick & Dirty. Тогда написаный код будет с душком и не будет соответствовать современным стандартам программирования

---

Возьмём за основу простое исполнение и посмотрим, что будет, если его порефакторить (код с коммитами)

public async Task<ActionResult<MeasurementSet>>
        Get([FromQuery]DateTime? from = null, 
            [FromQuery]DateTime? to = null)
{
	// Defaulting values
  from ??= DateTime.Today.AddDays(-1);
  to ??= DateTime.Today;
  // Defining URLs
  var query = $"beginn={from:dd.MM.yyyy}&ende={to:dd.MM.yyyy}";
  var baseUrl = new Uri("https://BaseURL/");
  using (var client = new HttpClient { BaseAddress = baseUrl })
  {
    // Collecting data
    return Ok(new MeasurementSet 
    { 
      Temperature = await GetMeasures(query, client, "wassertemperatur"), 
      Level = await GetMeasures(query, client, "wasserstand"), 
    });
  }
}

private static async Task<IEnumerable<Measurement>>
        GetMeasures(string query, HttpClient client, string apiName)
{
  // Retrieving the data
  var response = await client.GetAsync($"{apiName}/some/other/url/part/?{query}");
  var html = await response.Content.ReadAsStringAsync();
  // Parsing HTML response
  var bodyMatch = Regex.Match(html, "<tbody>(.*)<\\/tbody>");
  var rowsHtml = bodyMatch.Groups.Values.Last();
  return Regex.Matches(rowsHtml.Value, "<tr  class=\"row2?\"><td >([^<]*)<\\/td><td  class=\"whocares\">([^<]*)<\\/td>")
    // Building the results
    .Select(match => new Measurement
    {
      Date = DateTime.Parse(match.Groups[1].Value),
      Value = decimal.Parse(match.Groups[2].Value)
    });
}

1. Обработка ошибок

Код написан без учёта возможных проблем и если что-то пойдёт не так, то понять причину будет сложно. Добавим пару более подробных исключений

if (response.IsSuccessStatusCode)
{
  throw new Exception($"{apiName} gathering failed with. [{response.StatusCode}] {html}");
}
// Parsing HTML response
var bodyMatch = Regex.Match(html, "<tbody>(.*)<\\/tbody>");
if (!bodyMatch.Success)
{
  throw new Exception($"Failed to define data table body. Content: {html}");
}

2. Разделение слоёв

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

3. Принцип единственной ответственности

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

var apiName = measure switch
{
  MeasureType.Temperature => "wassertemperatur",
  MeasureType.Level => "wasserstand",
  _ => throw new NotImplementedException($"Measure type {measure} not implemented")
};

Так получается, что логика по составлению запроса на основании параметров совмещена с другими элементами. Выделим её в отдельный класс:

public class UrlQueryBuilder
{
  public DateTime From { get; set; } = DateTime.Today.AddDays(-1);
  public DateTime To { get; set; } = DateTime.Today;

  public string Build(MeasureType measure)
  {
    var query = $"beginn={From:dd.MM.yyyy}&ende={To:dd.MM.yyyy}";
    var apiName = measure switch
    {
      MeasureType.Temperature => "wassertemperatur",
      MeasureType.Level => "wasserstand",
      _ => throw new NotImplementedException($"Measure type {measure} not implemented")
    };
    return $"{apiName}/some/other/url/part/?{query}";
  }
}

4. Снижаем сопряжение (coupling)

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

var settings = Configuration.GetSection("AppSettings").Get<AppSettings>();
services.AddHttpClient(Constants.ClientName, client =>
{
  client.BaseAddress = new Uri(settings.BaseUrl);
});
services.AddTransient<IRawMeasuresCollector, RawMeasuresCollector>();

5. Тестирование

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

Конечно проект гибкийКонечно проект гибкий[TestMethod]
public async Task TestHtmlTemperatureParsing()
{
  var collector = new Mock<IRawMeasuresCollector>();
  collector
    .Setup(c => c.CollectRawMeasurement(MeasureType.Temperature))
    .Returns(Task.FromResult(_temperatureDataA));

  var actual = (await new MeasureParser(collector.Object)
    .GetMeasures(MeasureType.Temperature)
    ).ToArray();

  Assert.AreEqual(165, actual.Length);
  Assert.AreEqual(7.1M, actual
      .First(l => l.Date == DateTime.Parse("24.11.2020 10:15")).Value);
}

6. Повышаем гибкость

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

Итоговая функция контроллера примет вид:

public async Task<ActionResult<MeasurementSet>> Get
    ([FromQuery] DateTime? from = null, [FromQuery] DateTime? to = null)
{
  var parser = new MeasureParser(_collectorFactory.CreateCollector(from, to));
  return Ok(new MeasurementSet
  {
    Temperature = await parser.GetTemperature(),
    Level = await parser.GetLevel(),
  });
}

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

Итог

В результате одностраничный метод вырос в приличный проект

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

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

Поделитесь своим опытом.