Nel caso dell'esempio sopra, è ragionevole che un Car possa esistere senza CarRadio . In tal caso, direi che non solo è accettabile passare un CarRadio nullo a volte, direi che è obbligatorio. I test devono garantire che l'implementazione della classe Car sia solida e non genera eccezioni puntatore nullo quando non è presente CarRadio .

Tuttavia, prendiamo un esempio diverso: consideriamo un SteeringWheel . Supponiamo che un Car debba avere un SteeringWheel , ma al test della velocità non interessa molto. In questo caso, non vorrei passare un SteeringWheel nullo, in quanto questo è spingere la classe Car in posizioni in cui non è progettata per andare. In questo caso, ti conviene creare una sorta di DefaultSteeringWheel che (per continuare la metafora) è bloccato in linea retta.

Is constructing objects with null in unit tests a code smell?

Sì. Nota la definizione C2 di odore di codice : "un CodeSmell è un suggerimento che qualcosa potrebbe essere sbagliato, non una certezza."

The test runs fine. SpeedLimit needs a Car with a Motor in order to do its thing. It is not interested in a CarRadio at all, so I provided null for that.

Se stai provando a dimostrare che speedLimit.IsViolatedBy(car) non dipende dall'argomento CarRadio passato al costruttore, allora sarebbe probabilmente più chiaro al futuro manutentore rendere esplicito quel vincolo introducendo un test double che fallisce il prova se è invocato.

Se, come è più probabile, stai solo cercando di salvarti il lavoro di creare un CarRadio che sai non è usato in questo caso, dovresti notare che

• si tratta di una violazione dell'incapsulamento (stai lasciando che il fatto che CarRadio non sia stato utilizzato si perde nel test)
• hai scoperto almeno un caso in cui desideri creare un Car senza specificare CarRadio

Ho il strong sospetto che il codice stia cercando di dirti che vuoi un costruttore che assomiglia a

public Car(IMotor motor) {...}

e quindi quel costruttore può decidere cosa fare con il fatto che non c'è CarRadio

public Car(IMotor motor) {
    this(null, motor);
}

o

public Car(IMotor motor) {
    this( new ICarRadio() {...} , motor);
}

o

public Car(IMotor motor) {
    this( new DefaultRadio(), motor);
}

ecc.

Its about if passing null to something else while testing SpeedLimit. You can see that the test is called "SpeedLimitTest" and the only Assert is checking a method of SpeedLimit.

Questo è un secondo odore interessante: per testare SpeedLimit, devi costruire un'intera macchina intorno a una radio, anche se l'unica cosa a cui probabilmente interessa il controllo SpeedLimit è la velocità .

Questo potrebbe suggerire che SpeedLimit.isViolatedBy dovrebbe accettare un'interfaccia per i ruoli che la macchina implementa , piuttosto che richiedere un'intera macchina.

interface IHaveSpeed {
    float speed();
}

class Car implements IHaveSpeed {...}

IHaveSpeed è un nome davvero scadente; si spera che il tuo linguaggio di dominio abbia un miglioramento.

Con quell'interfaccia installata, il tuo test per SpeedLimit potrebbe essere molto più semplice

public void TestSpeedLimit()
{
    IHaveSpeed somethingTravelingQuickly = new IHaveSpeed {
        float speed() { return 10f; }
    }

    SpeedLimit speedLimit = new SpeedLimit();
    Assert.IsTrue(speedLimit.IsViolatedBy(somethingTravelingQuickly));
}

Is constructing objects with null in unit tests a code smell?

No

Assolutamente no. Al contrario, se NULL è un valore disponibile come argomento (alcune lingue potrebbero avere costrutti che non consentono il passaggio di un puntatore nullo), dovrebbe verificarlo.

Un'altra domanda è cosa succede quando passi NULL . Ma questo è esattamente su cosa è un test unitario: accertarsi che un risultato definito stia accadendo quando per ogni possibile input. E NULL è un potenziale input, quindi dovresti avere un risultato definito e dovresti fare un test per questo.

Quindi se la tua auto è supposta per essere costruibile senza una radio, dovresti scrivere un test per questo. Se non lo è ed è supposto lanciare un'eccezione, dovrebbe scrivere un test per questo.

In ogni caso, si dovrebbe scrivere un test per NULL . Sarebbe un odore di codice se lo avessi lasciato fuori. Ciò significherebbe che hai un ramo di codice non verificato che hai appena assunto sarebbe magicamente privo di errori.

Si noti che sono d'accordo con le altre risposte che il codice in prova potrebbe essere migliorato. Tuttavia, se il codice migliorato ha parametri che sono puntatori, passare NULL anche per il codice migliorato è un buon test. Vorrei un test per mostrare cosa succede quando passo NULL come motore. Questo non è un odore di codice, è l'opposto di un odore di codice. Sta testando.

Personalmente sarei riluttante a iniettare valori nulli. Infatti, ogni volta che aggiungo delle dipendenze a un costruttore, una delle prime cose che faccio è sollevare un'eccezione ArgumentNullException se quelle iniezioni sono nulle. In questo modo posso tranquillamente usarli all'interno della mia classe, senza dozzine di controlli nulli sparsi in giro. Ecco un modello molto comune. Si noti l'uso di readonly per garantire che le dipendenze impostate nel costruttore non possano essere impostate su null (o qualsiasi altra cosa) successivamente:

class Car
{
   private readonly ICarRadio _carRadio;
   private readonly IMotor _motor;

   public Car(ICarRadio carRadio, IMotor motor)
   {
      if (carRadio == null)
         throw new ArgumentNullException(nameof(carRadio));

      if (motor == null)
         throw new ArgumentNullException(nameof(motor));

      _carRadio = carRadio;
      _motor = motor;
   }
}

Con quanto sopra, potrei forse avere un test unitario o due, dove inserisco valori nulli, giusto per assicurarmi che i miei assegni null funzionino come previsto.

Detto questo, questo diventa un non-problema, una volta che fai due cose:

1. Programma su interfacce invece di classi.
2. Utilizza un framework di simulazione (come Moq per C #) per iniettare dipendenze fittizie invece di null.

Quindi per il tuo esempio avresti:

interface ICarRadio
{
   bool Tune(float frequency);
}

interface Motor
{
   bool SetSpeed(float speed);
}

...
var car = new Car(new Moq<ICarRadio>().Object, new Moq<IMotor>().Object);

Puoi trovare ulteriori dettagli sull'uso di Moq qui .

Naturalmente, se vuoi capirlo prima senza fare il mocking, puoi comunque farlo, purché tu stia utilizzando le interfacce. Basta creare un CarRadio e un motore fittizi come segue e iniettarli invece:

class DummyCarRadio : ICarRadio
{
   ...
}
class DummyMotor : IMotor
{
   ...
}

...
var car = new Car(new DummyCarRadio(), new DummyMotor())

Non è solo ok, è una ben nota tecnica di rottura delle dipendenze per ottenere il codice legacy in un'imbracatura di test. (Vedi "Lavorare efficacemente con il codice legacy" di Michael Feathers.)

Odora? Beh ...

Il test non ha odore. Il test sta facendo il suo lavoro. Dichiara "questo oggetto può essere nullo e il codice sotto test funziona ancora correttamente".

L'odore è nell'implementazione. Dichiarando un argomento del costruttore, dichiari che "questa classe ha bisogno di questa cosa per funzionare correttamente". Ovviamente, non è vero. Tutto ciò che è falso è un po 'puzzolente.

Facciamo un passo indietro ai primi principi.

A unit test tests some aspect of a single class.

Ci saranno comunque costruttori o metodi che prendono altre classi come parametri. Il modo in cui gestirli varia da caso a caso, ma l'installazione dovrebbe essere minima poiché sono tangenziali rispetto all'obiettivo. Potrebbero esserci scenari in cui il passaggio di un parametro NULL è perfettamente valido, ad esempio un'automobile senza radio.

Sto presumendo qui, che stiamo solo testando la linea di corse per iniziare (per continuare il tema dell'auto), ma potrebbe anche voler passare NULL ad altri parametri per controllare che qualsiasi codice difensivo e meccanismi di gestione delle eccezioni funzionano come richiesto.

Fin qui tutto bene. Tuttavia, cosa succede se NULL non è un valore valido. Bene, eccoti nel mondo oscuro di mock, stub, manichini e falsi .

Se tutto ciò sembra un po 'troppo da prendere, in realtà stai già utilizzando un dummy passando NULL nel costruttore di Car poiché è un valore che potenzialmente non verrà utilizzato.

Ciò che dovrebbe diventare chiaro abbastanza rapidamente, è che sei potenzialmente in grado di risolvere il tuo codice con molta manipolazione NULL. Se si sostituiscono i parametri di classe con le interfacce, si aprono anche le porte per il mocking e il DI etc, che rendono queste operazioni molto più semplici da gestire.

Dando un'occhiata al tuo design, suggerirei che Car sia composto da un set di Features . Una caratteristica può essere un CarRadio o EntertainmentSystem che può consistere in cose come CarRadio , DvdPlayer ecc.

Quindi, come minimo, dovresti costruire un Car con un set di Features . EntertainmentSystem e CarRadio sarebbero implementatori dell'interfaccia (Java, C # ecc.) di public [I|i]nterface Feature e questa sarebbe un'istanza del modello di progettazione Composite.

Pertanto, costruisci sempre un Car con Features e un test unitario non istanzia mai un Car senza Features . Anche se potresti prendere in considerazione una simulazione di BasicTestCarFeatureSet con una serie minima di funzionalità richieste per il tuo test di base.

Solo alcuni pensieri.

Giovanni