Как скрыть логику за классом, чтобы улучшить читаемость метода и класса рефакторинга, чтобы следовать SRP?

Question

У меня есть алгоритм для создания версии для объекта, а затем я сохраняю эту версию для объекта ниже 2:

1) Вариант

2) Категория

interface IEntityVersion
{
    string GetVersion();
}

public class EntityVersion : IEntityVersion
{
    public string GetVersion()
    {
        return null;
    }
}

public interface IVariant
{
    void Process(Variant model, string connectionString);
}

public abstract class BaseVariant : IVariant
{
    private readonly IEntityVersion _entityVersion = new EntityVersion();

    public void Process(Variant model, string connectionString)
    {
        try
        {
            Transform();
            string variantVersion = _entityVersion.GetVersion();
            using (var myConnection = new SqlConnection(connectionString))
            {
                myConnection.Open();
                using (var transaction = myConnection.BeginTransaction())
                {
                    try
                    {
                        VariantRepo.UpdateVariantVersion(
                          myConnection,
                          transaction, model.VariantId, variantVersion);

                        CategoryRepo.UpdateCategoryVariantMapping(
                                     myConnection,
                                     transaction, model.CategoryId, variantVersion);

                        transaction.Commit();
                    }
                    catch (Exception)
                    {
                        transaction.Rollback();
                        DeleteStep1Data();
                    }
                }
            }
        }
        catch (Exception)
        {
            //log error
        }
    }

    protected abstract void DeleteStep1Data();
    protected abstract void Transform();
}

public class Variant
{
    public int VariantId { get; set; }
    public int CategoryId { get; set; }
}

public class VariantRepo
{
    public static void UpdateVariantVersion(SqlConnection sqlConnection,
        SqlTransaction transaction, int variantId, string version)
    {
        //save logic here
    }
}

public class CategoryRepo
{
    public static void UpdateCategoryVariantMapping(SqlConnection sqlConnection,
        SqlTransaction transaction, int categoryId, string version)
    {
        //save logic here
    }
}

У меня есть 2 производных типа (AggregateCalculator а также AdditionCalculator) у каждого своя реализация Transform а также DeleteStep1Data методы.

public class AggregateCalculator : BaseVariant
{
    protected override void DeleteStep1Data() // Is it violating SRP ?
    {
        throw new NotImplementedException();
    }

    protected override void Transform()
    {
        throw new NotImplementedException();
    }
}

public class AdditionCalculator : BaseVariant
{
    protected override void DeleteStep1Data()// Is it violating SRP ?
    {
        throw new NotImplementedException();
    }

    protected override void Transform()
    {
        throw new NotImplementedException();
    }
}

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

Step1 а также Step2 синхронизированы, так что если есть ошибка в Step2Я называю DeleteStep1Data способ удалить все данные, сохраненные в Step1,

Также я чувствую, что мои 2 производных класса AggregateCalculator а также AdditionCalculator обрабатывают более 1 ответственности, т.е. выполняют преобразование, а также удаляют данные, хранящиеся в процессе преобразования, хотя я не уверен, верно ли это или нет.

Есть ли возможность рефакторинга кода выше для улучшения читабельности и обработки SRP?

Answer 1

Пытаюсь понять ваш вопрос

У вас есть сущность... когда сущность изменяется, вы хотите создать версию изменения для своей сущности. Мне непонятно, почему это изменение нужно отслеживать как по варианту, так и по категории?

Предположим, ваша сущность car и категории для этого объекта: Toyota, BMW а также Nissan. Теперь ваша сущность, скажем, "Toyota Corona с id = 123" изменена. Зачем нужно отслеживать изменение по категории? Разве вы не можете просто сказать, что объект с id = 123 изменился?

Нарушение СРП

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

У вас есть класс под названием AggregateCalculator, Я предполагаю, что основная ответственность этого класса - вычислить агрегацию, которая происходит в Transform()метод. Теперь вам нужно выполнить 2 шага внутриTransform(). Это не обязательно нарушение SRP... потому что на более высоком уровне ваш агрегатный калькулятор делает одно: вычисляет агрегирование.

Вы можете искать общие признаки нарушения SRP:

1. Согласно 2-му закону IoC Николы Маловича:

   Любой класс, имеющий более 3-х зависимостей, должен быть опрошен на предмет нарушения SRP.

2. Если размер вашего класса слишком велик, вам нужно подвергнуть его сомнению на предмет нарушения SRP.

Нарушение DRY

Оба ваших класса: AggregateCalculator а также AdditionCalculatorвыполните их расчет в 2 шага, шаг 1 и шаг 2... и у вас есть общий метод:DeleteStep1Data()в обоих классах, чтобы удалить шаг 1, если шаг 2 завершится неудачно... Я предполагаю реализациюDeleteStep1Data()отличается для каждого из этих классов, но я чувствую, что он по-прежнему содержит дублированный код (не DRY). Можно утверждать, что это также нарушает SRP, потому чтоAggregateCalculator отвечает за оба: вычисление агрегации и "зеркалирование транзакции БД" (это сложно сказать, не видя полного кода).

Похоже, что и шаг-1, и шаг-2 являются транзакциями БД, поэтому другим подходом было бы поместить оба шага в одну транзакцию БД... например, вы можете написать такую ​​хранимую процедуру:

CREATE PROCEDURE AggregateCalculationSP
AS
BEGIN
    BEGIN TRANSACTION t1
        BEGIN TRY
            -- do step 1
            -- do step 2
        END TRY
        BEGIN CATCH
            ROLLBACK TRANSACTION t1
        END CATCH
    COMMIT TRANSATION t1
END

Теперь ты можешь достать DeleteStep1Data() из вашего класса.