Sta sostituendo la chiamata a metodi di mutuo esclusivismo con una strategia di iniezione forzata?

Question

Sta sostituendo la chiamata a metodi di mutuo esclusivismo con una strategia di iniezione forzata?

#1 da (3 voti)
#2 da (0 voti)

2

Recentemente ho dato il consiglio di refactoring del codice sulla falsariga di

class Validator<T> : IValidator
{
    public ValidationResult Validate(Validatable thingToValidate)
    {
        if(typeof(T).Name == nameof(FooThing))
        {
            ValidateFoo(thingToValidate);
        }
        if(typeof(T).Name == nameof(BarThing))
        {
            ValidateBar(thingToValidate);
        }
        if(typeof(T).Name == nameof(QuxThing))
        {
            ValidateQux(thingToValidate);
        }
    }
}

in qualcosa di simile

class Validator<T> : IValidator
{
    Dictionary<string, IValidator> specificValidators; // may be injected

    public ValidationResult Validate(Validatable thingToValidate)
    {
        var specificValidator = GetValidatorFor(typeof(T).Name);
        return specificValidator.Validate(thingToValidate);
    }

    private IValidator GetValidatorFor(string name)
    {
        return specificValidators[name];
    }
}

Mentre tendo a sostenere che questo uso di generici è un uso improprio, ho cercato intenzionalmente di mantenere intatta l'interfaccia pubblica.

Il fondamento logico del refactoring del codice originale era che era piuttosto rigido e aveva troppe responsabilità (alla fine della giornata, la classe era responsabile della validazione di Foo-things, Bar-things e Qux-things). Più specificamente ho sostenuto che

L'OCP è stato danneggiato, dal momento che non è in grado di estendere la classe senza cambiarla
L'SRP è stato danneggiato per i motivi sopra indicati

Mentre penso che questi argomenti valgano per mantenere la classe mantenibile (se fosse stato aggiunto un solo caso di validazione, la modifica non avrebbe avuto un grande impatto, ma se fossero stati altri dieci casi, la classe avrebbe davvero ottenuto ingombrante, inoltre, l'aggiunta manuale dei casi è soggetta a errori), ma sicuramente quest'ultima versione è decisamente meno leggibile.

Ammetto che non sono sicuro se si tratta di sovrastrutture eccessive o se questo refactoring è valido e richiesto per mantenere un'elevata manutenibilità. È?

clean-code solid single-responsibility open-closed-principle

posta Paul Kertscher 06.03.2018 - 11:19

fonte

2 risposte

Leggi altre domande sui tag clean-code solid single-responsibility open-closed-principle

Tieni i test unitari comuni in una libreria separata o duplicata? L'URL 'utenti / nuovi' RESTful?

score 3 · Answer 1

Il tuo Validator è sia una factory che crea un Validator che un Validator. Questo fa ancora troppo.

Quello che faccio normalmente è creare un validatore specifico per oggetto e iniettarne uno corretto.

Esempio:

public UserController
{
  private IValidator<User> _userValidator;
  public UserController(IValidator<User> validator)
  {
    _userValidator = validator;
  }
}

public Validator<User> : IValidator<User>
{
  public ValidationResult Validate(User user)
  { 
  }
}

È quindi possibile passare il validatore corretto attraverso una fabbrica o attraverso un Injection Framework registrando il corretto IValidator < Utente >.

L'aggiunta di un nuovo Validatore è quindi la creazione di uno nuovo e la registrazione in fabbrica o Injection Framework.

score 0 · Answer 2

Sono quasi d'accordo con Carra. Il tuo approccio è giudicato e non ti dà tutto ciò che desideri, Carra's uccide il tandem Validator-Validatable.

Preferirei vedere qualcosa di simile a questo:

abstract class Validator : IValidator
{
    public ValidationResult Validate(Validatable thingToValidate)
    {
        return DoValidate(thingToValidate);
    }

    protected abstract ValidationResult DoValidate(Validatable thingToValidate);

    protected void CheckType(Validatable thingToValidate, Type type)
    {
        if (typeof(thingToValidate) != type)
        {
            throw new ArgumentException("Wrong type.");
        }

    }
}

class FooValidator : Validator
{
    protected override ValidationResult DoValidate(Validatable thingToValidate)
    {
        CheckType(thingToValidate, typeof(Foo));

        // if this were not a direct descendant of Validator, you would call
        // base.DoValidate(thingToValidate);

        // Foo specific validation
    }
}

Una classe di validatori per ogni tipo validabile che discende da un validatore di classi astratte. In questo modo

rispetti l'interfaccia esistente;
ottieni SRP;
ottieni l'OCP;
non esiste un uso abusivo di generici.