Рефакторинг классов сервисного уровня

Question

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

public class InvoiceCalculator:IInvoiceCalculator
{
   public CalculateInvoice(Invoice invoice)
   {
      foreach (InvoiceLine il in invoice.Lines)
      {
          UpdateLine(il);
      }
      //do a ton of other stuff here
   }

   private UpdateLine(InvoiceLine line)
   {
      line.Amount = line.Qty * line.Rate;
      //do a bunch of other stuff, including calls to other private methods
   }
}

В этом упрощенном случае (по сравнению с классом из 1000 строк, который имеет 1 открытый метод и ~ 30 закрытых), мой босс говорит, что я должен иметь возможность тестировать мои CalculateInvoice и UpdateLine отдельно (UpdateLine фактически вызывает 3 других закрытых метода) и также выполняет вызовы из базы данных). Но как бы я это сделал? Его предложенный рефакторинг показался мне немного запутанным:

//Tiny part of original code
public class InvoiceCalculator:IInvoiceCalculator
{
   public ILineUpdater _lineUpdater;

   public InvoiceCalculator (ILineUpdater lineUpdater)
   {
      _lineUpdater = lineUpdater;
   }

   public CalculateInvoice(Invoice invoice)
   {
      foreach (InvoiceLine il in invoice.Lines)
      {
          _lineUpdater.UpdateLine(il);
      }
      //do a ton of other stuff here
   }
}

public class LineUpdater:ILineUpdater
{
   public UpdateLine(InvoiceLine line)
   {
      line.Amount = line.Qty * line.Rate;
      //do a bunch of other stuff
   }
}

Я вижу, как теперь нарушается зависимость, и я могу протестировать обе части, но это также создаст 20-30 дополнительных классов из моего исходного класса. Мы рассчитываем счета-фактуры только в одном месте, так что эти части не могут быть использованы повторно. Это правильный способ сделать это изменение, или вы предлагаете мне сделать что-то другое?

Спасибо!

Jess

Answer 1

Это пример Зависть к функциям :

line.Amount = line.Qty * line.Rate;

Вероятно, это должно выглядеть примерно так:

  var amount = line.CalculateAmount();

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

Answer 2

Ну, ваш босс идет более правильным путем с точки зрения юнит-тестирования:
Теперь он может тестировать CalculateInvoice () без тестирования функции UpdateLine (). Он может передать фиктивный объект вместо реального объекта LineUpdater и протестировать только CalculateInvoice (), а не целый набор кода.
Это правильно? Это зависит. Ваш босс хочет сделать реальные юнит-тесты. И тестирование в вашем первом примере не будет юнит-тестированием, это будет интеграционное тестирование.

Каковы преимущества юнит-тестов перед интеграционными тестами?
1) Модульные тесты позволяют тестировать только один метод или свойство, без влияния других методов / базы данных и т. Д.
2) Второе преимущество - модульные тесты выполняются быстрее (например, вы сказали, что UpdateLine использует базу данных), потому что они не тестируют все вложенные методы. Вложенные методы могут быть вызовами базы данных, поэтому, если у вас есть тысячи тестов, ваши тесты могут выполняться медленно (несколько минут).
3) Третье преимущество: если ваши методы выполняют вызовы из базы данных, иногда вам нужно настроить базу данных (заполнить ее данными, необходимыми для тестирования), и это может быть непросто - возможно, вам придется написать пару страниц кода просто подготовить базу данных для теста. С помощью модульных тестов вы отделяете вызовы базы данных от тестируемых методов (используя фиктивные объекты).

Но! Я не говорю, что юнит-тесты лучше. Они просто разные. Как я уже сказал, модульные тесты позволяют вам тестировать блок изолированно и быстро. Интеграционные тесты проще и позволяют тестировать результаты совместной работы разных методов и слоев. Честно говоря, я больше предпочитаю интеграционные тесты:)

Также у меня есть пара предложений для вас:
1) Я не думаю, что наличие поля Amount - хорошая идея. Кажется, что поле Amount является дополнительным, потому что его значение можно рассчитать на основе 2 других открытых полей. Если вы все равно хотите это сделать, я бы сделал это как свойство только для чтения, которое возвращает Qty * Rate.
2) Обычно наличие класса, состоящего из 1000 строк, может означать, что он плохо спроектирован и должен быть реорганизован.

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

Answer 3

IMO, все это зависит от того, насколько «значительным» является метод UpdateLine (). Если это просто деталь реализации (например, она может быть легко встроена в метод CalculateInvoice () и единственное, что может повредить читабельности), то вам, вероятно, не нужно тестировать ее отдельно от мастер-класса.

С другой стороны, если метод UpdateLine () имеет какое-то значение для бизнес-логики, если вы можете представить себе ситуацию, когда вам нужно будет изменить этот метод независимо от остальной части класса (и, следовательно, протестировать его отдельно), тогда вам следует продолжить его рефакторинг в отдельный класс LineUpdater.

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

Answer 4

Подход вашего помощника является отличным примером внедрения зависимости, и как это позволяет вам использовать макет ILineUpdater для эффективного проведения ваших тестов.

Answer 5

Пример твоего босса выглядит для меня разумным.

Некоторые ключевые соображения, которые я стараюсь учитывать при разработке любого сценария:

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

   Класс должен меняться только по одной причине.

2. Оправдывает ли каждый новый класс свое существование

   Созданы ли классы только ради этого или они содержат значимую часть логики?

3. Можете ли вы протестировать каждый фрагмент кода изолированно?

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

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

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

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

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

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

Answer 6

да, хороший. Я не уверен, что в объект InvoiceLine также включена некоторая логика, в противном случае вам, вероятно, также понадобится IInvoiceLine. У меня иногда одни и те же вопросы. С одной стороны, вы хотите сделать все правильно и выполнить модульное тестирование своего кода, но когда речь идет о вызовах базы данных и, возможно, написании файлов, это требует много дополнительной работы для настройки первого теста со всеми объектами тестирования, которые выполняются, когда речь идет о написании файлов и базе данных. случается, интерфейсы, утверждает, и вы также хотите проверить, что уровень данных не содержит ошибок. Таким образом, тест, который является больше «процессом», чем «модулем», часто легче построить. Если у вас есть проект, который будет сильно изменен (в будущем) и множество зависимостей этого кода (возможно, другие программы читают данные файла или базы данных), было бы неплохо провести твердый модульный тест для всех частей вашего кода. и время инвестиций стоит. Но если проект будет, как мой последний клиент говорит: «Давайте запустим его, и, возможно, мы немного подправим в следующем году, и в следующем году будет что-то новое», то я бы не стал так усердно брать на себя всю работу. тестирование и запуск.

Michel