Controllo del risultato di un costruttore in C #

Sono d'accordo con @MartinMaat sulla scelta delle tue battaglie.

Ci sono molti casi di "just in case" a causa della scarsa comprensione della lingua nonostante il linguaggio sia fissato nelle sue regole per molte di queste cose - oltre a parentesi di un'espressione che non ne ha bisogno a causa della mancata comprensione del regole di precedenza della lingua. Ma ancora, tale pratica è per lo più innocua.

Quando ero più giovane, sentivo che dovevamo imparare i dettagli della lingua e quindi evitare di scrivere un codice così superfluo. (Uno dei miei pet-peeves era return (0); con i suoi paramenti superflui.) Tuttavia, ora moderatore quella posizione, in particolare, perché usiamo così tante lingue diverse ora, saltando da client a server, ecc ... Quindi, io ora riduciamo il gioco per alcuni di questi problemi.

Il punto sul ciclomatico inizia ad andare su argomenti logicamente ragionati. Diamo un'occhiata a Copertura del codice e in particolare livelli più elevati di copertura :

2. Each decision takes every possible outcome

Poiché non possiamo forzare la nuova operazione a restituire NULL, non c'è modo di raggiungere livelli più elevati di copertura del codice per questa operazione condizionale. Naturalmente, questo può non essere importante per la tua organizzazione!

Tuttavia, a causa di questo problema di copertura del codice, lo assegnerei una priorità più alta rispetto alla sovra-parentesi.

D'altro canto, il codice generato sottostante non subirà probabilmente un bit per questo poiché le generazioni di codice, JIT e gli ottimizzatori comprendono tutti che un valore new ed non sarà mai nullo. Quindi, il costo reale viene fornito solo in termini di leggibilità e capacità di copertura del codice sorgente.

Vorrei chiederti a cosa assomiglia la "parte opposta" di una simile affermazione?

Se non c'è altra parte, direi che semplicemente cadere alla fine della routine o cadere in un altro codice (cioè non else per questa if ) è potenzialmente pericoloso, poiché ora questo "solo nel caso in cui "logicamente suggerisce che i chiamanti e / o ulteriore codice lungo la linea gestisce anche NULL.

Se legge:

p = new Object ();
if ( p != null ) {
    p.field = value;
}
else {
    throw new NullReferenceException ();
}

allora questo è davvero eccessivo, poiché il linguaggio fa tutto questo per noi.

Potrei suggerire di invertire il senso del condizionale - forse il tuo collega sarà più a suo agio in questo:

p = new Object ();
if ( p == null ) {
    throw new NullReferenceException ();
}
else {
    p.field = value;
}

Ora puoi discutere della rimozione del wrapper else, poiché è molto chiaramente non necessario:

p = new Object ();
if ( p == null ) {
    throw new NullReferenceException ();
}
p.field = value;

Con questo, il "just in case" ora è ciò che è condizionale, mentre il codice successivo non lo è. Questo approccio rafforza ulteriormente il fatto che quando l'allocazione fallisce, la risposta appropriata viene lanciata, piuttosto che continuare a eseguire il codice in questo metodo e / o in questa catena di chiamate (senza alcuna altra corretta gestione dell'errore di allocazione).

Quindi, in sintesi ci sono due argomenti logicamente ragionati per fare qui contro questa pratica:

1. Non è possibile raggiungere livelli di copertura del codice più elevati poiché non è possibile forzare la memoria esaurita (o qualsiasi errore del costruttore) per restituire null.
2. Il "just in case" (come mostrato sopra nella domanda) è incompleto e come tale è viziato a causa dell'incongruenza delle aspettative su come dovevano essere nulle gestito da altro codice oltre / passato il p.field = value; .

Fondamentalmente, sembra che forse il tuo collega stia cercando di usare le eccezioni, anche se in C # non c'è scelta per queste cose. ( Se vogliamo codice ben testato non possiamo codificare sia per un modello di eccezione per la gestione di null e un modello non-eccezione che utilizza valori di ritorno null, affiancati.) Forse se ragionate con il tuo collega attraverso questi argomenti, vedranno un po 'di luce!

Forse puoi evitare il problema e semplicemente fargli sapere che esiste una sintassi c # più recente che è più facile (e fa il controllo null per te!)

Invece di

Person p = new Person();
if (p != null)
{
    p.Name = "John Smith";
}

Scrivi

var p = new Person
{
    Name = "John Smith"
};

È una richiesta ragionevole ma sembra che questo sia già diventato una lotta tra te e lui. Potresti averlo già fatto notare a lui, è riluttante a cambiarlo e ora hai in programma di intensificare la questione. Quindi puoi "vincere".

Potrebbe essere più semplice mandargli il link che Robert Harvey ha pubblicato e dire "ti ho mandato un link sulla costruzione di oggetti C # che potresti trovare interessante" e lasciarlo lì. Non è esattamente dannoso quello che fa (solo inutile) e facile da pulire op dopo. Sto dicendo: scegli le tue battaglie. Sono stato in questa situazione più di una volta e, a posteriori, spesso non valeva la pena lottare. Vieni troppo facilmente a odiarti a vicenda praticamente su nulla. Sì, hai ragione, ma mentre sei solo tu e lui, potresti voler mantenere un terreno comune.

Poiché la tua domanda tecnica è già stata risolta (il controllo nulla è inutile), vorrei concentrarmi su un aspetto diverso della tua domanda: come affrontare una simile situazione in generale: un collaboratore che non conosce alcuni fatti di base sugli strumenti che usi tutti e quindi utilizza gli strumenti in modo non ottimale.

In alcuni casi, potrebbe non interessarti molto. Se fanno il loro lavoro e non interferiscono con il tuo, puoi semplicemente ignorare il problema.

In altri casi (ad es. il tuo), fanno interferiscono con il tuo lavoro, almeno in una certa misura. Cosa fare al riguardo?

Penso che dipenda da molti dettagli. Ad esempio, per quanto tempo tu e il tuo collega siete stati con l'azienda, in che modo l'azienda si occupa in generale di tali problemi, quanto la questione influisce / vi infastidisce, quanto siete d'accordo con il collega, quanto è importante un'armoniosa relazione con il tuo collega, fino a che punto sollevare o intensificare il problema influisce su questa relazione, ecc.

La mia opinione personale potrebbe essere questa: penso che gli ingegneri dovrebbero conoscere i loro strumenti e gli ingegneri del software dovrebbero conoscere i loro linguaggi di programmazione. Naturalmente, nessuno conosce ogni dettaglio, ma i costruttori non restituiscono mai null è un'idea molto semplice - tutti dovrebbero saperlo. Quando ho bisogno di lavorare su un codice che contenga assegni null inutili, li rimuoverò semplicemente. Quando un collega mi chiede perché li ho rimossi, spiegherei perché sono inutili. (Se necessario, spiegherei anche perché il codice che è del tutto inutile dovrebbe essere rimosso.) Nel lungo periodo, ciò porterebbe a un codice che è privo di questi inutili controlli nulli.

Può sembrare un po 'arrogante e potrebbe non essere l'approccio giusto per tutti. Mi è stato detto che sono paziente e bravo a spiegare le cose. Forse è per questo che tendo a farla franca con questo approccio senza imbattersi in un coglione. : -)

Hai menzionato "una richiesta formale che questa pratica venga interrotta". Non ho mai lavorato in una società che avesse regole formali così dettagliate, ma qui ci sono alcuni pensieri comunque. Ancora una volta, penso che la tua migliore linea d'azione dipenda dai dettagli. Se presentare una richiesta del genere renderebbe il tuo collega un cattivo aspetto agli occhi di colleghi o dirigenti, probabilmente non lo farei. D'altra parte, se tali richieste vengono generalmente considerate come un gradito miglioramento e gli altri non incolpano il tuo collaboratore di non aver seguito prima questa regola, vai avanti. Nessuno sarà danneggiato, tutti staranno meglio.

Ma se non riuscissi a trovare un buon modo per affrontare il problema, potrei ricorrere al sarcasmo e aggiungere codice come questo ovunque:

int c = a + b;
if (c == a + b)
{
    // next steps
}
else 
{
    throw new ArithmeticException();
}

È altrettanto inutile come un controllo nullo dopo una chiamata al costruttore, ma ancora più ovvio. E ovviamente, non lo farei davvero. A meno che non sia così seccato con i miei colleghi che sto pensando di smettere comunque. ; -)