Nome / Correzione per codice di produzione Il cui unico scopo è facilitare i test? [duplicare]

11

La domanda "duplicata" collegata è al meglio una partita incerta, perché sta chiedendo

  • è pattern X OK ( / NO )

e chiaramente sono già nel campo NO e successivamente chiedo

  • che cos'è pattern X chiamato
  • quali passi possono essere presi per correggere pattern X

(nessuno dei quali è indirizzato dalla domanda collegata).

Recentemente ho eseguito una revisione del codice su un blocco di codice simile a questo:

public class MyClass
{
    private ISomething mySomething;
    // ...Other variables omitted for brevity

    public MyClass() { mySomething = new Something();  }

    /// <summary>
    /// Constructor - ONLY USE THIS FOR UNIT TESTING
    /// </summary>
    public MyClass(ISomething something) { mySomething = something; }

    public void MyMethod()
    {
        // Gets called by the framework, and changes the internal state of the class by using mySomething...
    }

    // Other methods...
}

Sono preoccupato specificamente con il costruttore sovraccarico. È stato aggiunto esclusivamente per testare questa classe e si farà strada nel codice di produzione.

Esiste un nome per questo pattern / anti-pattern e cosa si può fare per risolverlo?

Per maggiore chiarezza, l'implementazione di Something è stata aggiunta specificamente allo scopo di poter aggiungere un costruttore sovraccarico a MyClass . È usato da nessuna parte altro. La sua esistenza è un esempio del problema che mi preoccupa.

ISomething è strettamente correlato a MyClass . Non è necessario che sia stato estratto. Anche l'implementazione e l'interfaccia potrebbero assomigliare a:

public interface ISomething
{
    string GetClassName();
}

public class Something : ISomething
{
    public string GetClassName() { return "MyClass"; }
}

Ciò significa che il corpo di MyClass.MyMethod() potrebbe essere sostituito con return "MyClass";

Tuttavia, l'uso improprio dell'interfaccia / ottimizzazione prematura sembra un problema separato e non nello spirito della domanda originale (vale a dire considerare un dato di fatto che la classe / interfaccia sia strutturata in questo modo e lasciarla come separata [ma valida] preoccupazione).

    
posta Michael 20.11.2014 - 17:26
fonte

7 risposte

26

Per i metodi di una classe che sono esclusivamente a scopo di test, in passato ho visto il nome sportello di manutenzione . E simili ai veri e propri portelli di manutenzione nelle macchine fisiche, quei metodi a volte hanno il loro scopo. Ad esempio, se si renderà verificabile un codice legacy quando è diventato troppo grande dopo alcuni anni di evoluzione, i portelli di manutenzione possono essere di grande valore.

Ma concordo anche con le altre risposte qui, tali metodi dovrebbero essere un'eccezione e quando si mantengono piccole classi e componenti, con interfacce ben progettate, raramente ne hai bisogno.

    
risposta data 20.11.2014 - 17:43
fonte
16

It was added purely to test this class, and will make its way into production code.

Questo è miope ...

Avere un costruttore che passa le dipendenze non viene fatto solo per testare la classe. È fatto per rendere flessibile la tua classe. Il costruttore senza parametri che ha una dura dipendenza su un% diSomething concreto è più dell'anti-pattern dovuto all'accoppiamento stretto, ed è aggiunto solo per comodità del programmatore.

    
risposta data 20.11.2014 - 17:32
fonte
12

Si chiama "codice di produzione incluso al solo scopo di facilitare i test."

Se lo stai usando molto, direi che è un anti-modello. Il modo in cui lo si attenua è scrivere le classi utilizzando dipendenze conformi a un'interfaccia e fornite con Iniezione di dipendenza, quindi utilizzare stub e mock per isolare la classe per il test.

Ma la realtà è che è difficile isolare alcune classi. Nonostante la sua reputazione di testabilità, ASP.NET MVC ha alcune funzionalità come HttpContext che sono notoriamente difficili da prendere in giro. Le classi progettate per eseguire la gestione dei file possono essere difficili da testare senza codice helper, in quanto è necessario configurare scenari elaborati di file e cartelle per testarli. Quindi posso vedere motivi legittimi per includere tale codice.

    
risposta data 20.11.2014 - 17:34
fonte
7

Un termine che è stato usato di recente è danno al design indotto da test .

Telastyn è corretto che il tuo frammento di codice non sia in realtà un ottimo esempio di questo concetto, comunque.

Un evento più comune è che un test vuole impostare una classe in uno stato particolare durante la sua fase di "organizzazione", ma a causa dell'incapsulamento, non ha alcun modo di farlo direttamente. Quindi il test deve scegliere di eseguire una serie di passaggi per portare l'oggetto allo stato desiderato (il che richiede di abbinare il test a molto più della semplice dichiarazione che sta tentando di verificare) o di interrompere l'incapsulamento. La scelta di quest'ultima opzione richiede modifiche potenzialmente pericolose al codice di produzione.

    
risposta data 20.11.2014 - 17:32
fonte
3

Per quanto ne so, non esiste un nome generico per il codice di produzione creato appositamente ai fini dei test unitari. Con la notevole eccezione di amici-assiemi (per unità interne che testano unità) tale codice normalmente essere un odore che indica che qualcosa sta nella forma della firma pubblica della tua classe.

Questo esempio particolare , tuttavia, ha un nome specifico. Si chiama "Poor Man's Iniezione delle dipendenze "(o talvolta" Bastard Injection ") per il modo in cui assomiglia a dipendenza appropriata iniezione ma in realtà non interrompe l'accoppiamento tra il chiamante e la dipendenza.

L'anti-pattern qui non è: -

public MyClass(ISomething something) { mySomething = something; }

L'anti-pattern qui è: -

public MyClass() { mySomething = new Something(); }

Ora, non è necessariamente il caso che qualsiasi cosa abbia riparato immediatamente qui. Potresti essere ok a lasciarlo scorrere una volta, ma se dovesse succedere spesso ti conviene rimuovere il costruttore senza parametri e avere MyClass sempre ricevere la sua dipendenza dal codice esterno - possibilmente con l'aiuto di un IoC Container o ServiceLocator .

(Nota: sebbene i Container IoC siano molto più di moda dei ServiceLocators al giorno d'oggi, l'importante è separare la configurazione e l'utilizzo. L'uso di entrambi è meglio che non usare nessuno dei due.)

Rispondendo alla tua modifica: -

Come ho detto sopra, le difficoltà di test sono normalmente indicative di un design scadente. Puoi sistemarlo in due modi. O puoi aprire alcune porte per il codice di test (in modo inappropriato, come lo metti tu) per accedere ai componenti interni di un oggetto altrimenti impossibile da testare o risolvere il problema di progettazione sottostante.

Ora, questo è un po 'strong, perché la correlazione tra "codice ben progettato" e "codice facile da testare" non è abbastanza 100%. Nel caso che hai richiamato nel tuo OP, il problema di progettazione sembra ovvio (l'oggetto istanzia la sua violazione di DIP proprio - >, anche se a un esame più attento può anche accadere che la dipendenza non sia necessaria). A volte il problema del design è meno ovvio, o - specialmente nel codice legacy in cui il refactoring architettonico è difficile e rischioso - non vale la pena risolverlo. Dovrai usare il tuo giudizio per capire quale è quale.

    
risposta data 21.11.2014 - 15:31
fonte
0

Due possibili casi in cui potresti voler cambiare il codice di produzione a scopo di test:

  1. Se hai funzionalità complesse nella classe
    1. E la tua interfaccia aperta fa parecchie cose combinate
    2. quindi non è possibile testare separatamente ciascuna funzionalità
    3. questo significa che quando un test combinato fallisce, non si sa ancora quale parte è rotta
    4. Sì, è bene separare tale funzionalità in sottoclassi e testarle
  2. Se vuoi testare l'implementazione interna
    1. Questo frena i principi di refactoring / incapsulamento perché
    2. la tua logica di business non frenerà con la nuova implementazione
    3. ma i tuoi test freneranno, che è BAD poiché i test dovrebbero verificare il comportamento degli oggetti dal punto di vista dell'utente.

Quindi la risposta è che l'anti-pattern (item 2) rompe logicamente l'intenzione e lo scopo dell'incapsulamento. Io voto per la violazione del principio di incapsulamento .

    
risposta data 21.11.2014 - 08:18
fonte
-2

Supponendo che tu non accetti l'argomento (che credo valga la pena considerare) che il fatto che l'ISomething separato sia richiesto per i test ti dice che potrebbe essere necessario kn futuro per rendere la tua classe più flessibile, una possibile correzione sarebbe quello di contrassegnare il costruttore come deprecato e disabilitare gli avvisi di deprecazione per la riga della classe di test che lo utilizza. Questo sembra molto meglio di quello interno, in quanto ciò fa ben poco per impedire l'uso effettivo.

    
risposta data 21.11.2014 - 10:00
fonte

Leggi altre domande sui tag