Как я могу улучшить этот дизайн?

Question

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

public abstract class BaseBusinessAction<TActionParameters> 
    : where TActionParameters : IActionParameters
{
    protected BaseBusinessAction(TActionParameters actionParameters)
    {
        if (actionParameters == null) 
            throw new ArgumentNullException("actionParameters"); 

        this.Parameters = actionParameters;

        if (!ParametersAreValid()) 
            throw new ArgumentException("Valid parameters must be supplied", "actionParameters");
    }

    protected TActionParameters Parameters { get; private set; }

    protected abstract bool ParametersAreValid();

    public void CommonMethod() { ... }
}

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

Итак, вопрос такой: как мне улучшить этот дизайн?

Добавлять ли параметр Func<bool, TActionParameters> в конструктор базового класса? Если бы я сделал:

public class MyAction<MyParameters>
{
    public MyAction(MyParameters actionParameters, bool something) : base(actionParameters, ValidateIt)
    {
        this.something = something;
    }

    private bool something;

    public static bool ValidateIt()
    {
       return something;
    }

}

Это сработает, потому что ValidateIt статично, но я не знаю ... Есть ли лучший способ?

Комментарии приветствуются.

Answer 1

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

Итак, у вас есть несколько вариантов здесь:

1. Игнорировать проблему. Если вы считаете, что разработчики должны иметь возможность писать метод проверки параметров, не полагаясь ни на какое состояние класса во время выполнения, то запишите это предположение и придерживайтесь своего проекта.
2. Переместите логику проверки в каждый конструктор производного класса, пусть базовый класс выполняет только самые основные, абстрактные виды проверок, которые он должен (нулевые проверки и т. Д.).
3. Дублируйте логику в каждом производном классе. Этот тип дублирования кода кажется тревожным, и он открывает двери для производных классов, чтобы забыть выполнить необходимую настройку или логику проверки.
4. Предоставьте метод Initialize () некоторого вида , который должен вызываться потребителем (или фабрикой для вашего типа), чтобы гарантировать, что эта проверка будет выполнена после того, как тип полностью создан. Это может быть нежелательно, поскольку требует, чтобы каждый, кто создает экземпляр вашего класса, не забывал вызывать метод инициализации, который, как вы думаете, мог бы выполнить конструктор. Зачастую Factory может помочь избежать этой проблемы - она ​​будет единственной, которой разрешено создавать экземпляры вашего класса, и будет вызывать логику инициализации перед возвратом типа потребителю.
5. Если валидация не зависит от состояния, затем разделить валидатор на отдельный тип, который можно даже сделать частью сигнатуры родового класса. Затем вы можете создать экземпляр валидатора в конструкторе, передать ему параметры. Каждый производный класс может определять вложенный класс с конструктором по умолчанию и помещать туда всю логику проверки параметров. Пример кода этого шаблона приведен ниже.

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

Вот пример кода:

public interface IParamValidator<TParams> 
    where TParams : IActionParameters
{
    bool ValidateParameters( TParams parameters );
}

public abstract class BaseBusinessAction<TActionParameters,TParamValidator> 
    where TActionParameters : IActionParameters
    where TParamValidator : IParamValidator<TActionParameters>, new()
{
    protected BaseBusinessAction(TActionParameters actionParameters)
    {
        if (actionParameters == null) 
            throw new ArgumentNullException("actionParameters"); 

        // delegate detailed validation to the supplied IParamValidator
        var paramValidator = new TParamValidator();
        // you may want to implement the throw inside the Validator 
        // so additional detail can be added...
        if( !paramValidator.ValidateParameters( actionParameters ) )
            throw new ArgumentException("Valid parameters must be supplied", "actionParameters");

        this.Parameters = actionParameters;
    }
 }

public class MyAction : BaseBusinessAction<MyActionParams,MyActionValidator>
{
    // nested validator class
    private class MyActionValidator : IParamValidator<MyActionParams>
    {
        public MyActionValidator() {} // default constructor 
        // implement appropriate validation logic
        public bool ValidateParameters( MyActionParams params ) { return true; /*...*/ }
    }
}

Answer 2

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

Для меня я обычно ставлю этот вид проверки в начале любого вызываемого метода. Например,

public MyAction(MyParameters actionParameters, bool something)
    : base(actionParameters) 
{ 
    #region Pre-Conditions
        if (actionParameters == null) throw new ArgumentNullException();
        // Perform additional validation here...
    #endregion Pre-Conditions

    this.something = something; 
} 

Надеюсь, это поможет.

Answer 3

Я бы порекомендовал применить Принцип единой ответственности к проблеме. Кажется, что класс Action должен отвечать за одну вещь; выполнение действия. Учитывая это, валидация должна быть перемещена в отдельный объект, который отвечает только за валидацию. Вы можете использовать некоторый универсальный интерфейс, такой как этот, чтобы определить валидатор:

IParameterValidator<TActionParameters>
{
    Validate(TActionParameters parameters);
}

Затем вы можете добавить это в базовый конструктор и вызвать там метод validate:

protected BaseBusinessAction(IParameterValidator<TActionParameters> validator, TActionParameters actionParameters)
{
    if (actionParameters == null) 
        throw new ArgumentNullException("actionParameters"); 

    this.Parameters = actionParameters;

    if (!validator.Validate(actionParameters)) 
        throw new ArgumentException("Valid parameters must be supplied", "actionParameters");
}

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

Answer 4

В прошлом у меня была очень похожая проблема, и в итоге я переместил логику для проверки параметров в соответствующий класс ActionParameters. Этот подход будет работать сразу после установки, если ваши классы параметров будут выровнены с классами BusinessAction.

Если это не так, это становится более болезненным. У вас есть следующие варианты (лично я бы предпочел первый):

1. Обернуть все параметры в IValidatableParameters. Реализации будут выровнены с бизнес-действиями и обеспечат проверку
2. Просто отключите это предупреждение
3. Переместите эту проверку в родительские классы, но затем вы получите дублирование кода
4. Переместите эту проверку в метод, который фактически использует параметры (но затем ваш код завершится ошибкой)

Answer 5

Как насчет перемещения проверки в более распространенное место в логике. Вместо запуска проверки в конструкторе, запустите ее при первом (и только первом) вызове метода. Таким образом, другие разработчики могут сконструировать объект, а затем изменить или исправить параметры перед выполнением действия.

Вы можете сделать это, изменив свой метод получения / установки для свойства Parameters, чтобы все, что использует параметры, проверяло их при первом использовании.

Answer 6

Где предполагается использовать параметры: из CommonMethod? Непонятно, почему параметры должны быть действительными во время создания экземпляра, а не во время использования, и, таким образом, вы можете оставить его до производного класса для проверки параметров перед использованием.

РЕДАКТИРОВАТЬ - Учитывая то, что я знаю, проблема, кажется, одна из специальных работ, необходимых для создания класса. Для меня это говорит о классе Factory, используемом для создания экземпляров BaseBusinessAction, в котором он будет вызывать виртуальный Validate () для экземпляра, который он создает при его создании.

Answer 7

Почему бы не сделать что-то вроде этого:

public abstract class BaseBusinessAction<TActionParameters> 
    : where TActionParameters : IActionParameters
{

    protected abstract TActionParameters Parameters { get; }

    protected abstract bool ParametersAreValid();

    public void CommonMethod() { ... }
}

Теперь конкретному классу приходится беспокоиться о параметрах и обеспечении их валидности. Я бы просто принудил CommonMethod вызывать метод ParametersAreValid, прежде чем делать что-либо еще.