Non penso che la domanda collegata abbia una risposta che affronta il tuo caso specifico quindi farò inserisci un'altra risposta qui.
Lo snippet di codice che hai mostrato contiene due chiamate a Framework.NullCheck con uno scopo molto diverso.
Parametri
Il primo convalida un parametro. Dobbiamo testarlo? La prima domanda qui dovrebbe essere ciò che dice il contratto della funzione UpdateUser . Non hai mostrato la sua documentazione quindi non lo so.
Interfacce con contratti estesi
Se la documentazione dice
If userArg is null, a NullPointerException is thrown and the function has no effect.
allora sì, devi assolutamente controllarlo. Diciamo che una tale interfaccia ha un ampio contratto . Bene, in questo caso, il controllo è effettivamente ridondante perché se userArg == null allora la prima riga di codice
var user = userDb.Get(userArg.Id);
farà esattamente ciò che il contratto pubblicizza perché la JVM valuterà per prima cosa i parametri della funzione e la valutazione di userArg.Id è garantita per innescare un NullPointerException senza effetti collaterali. Potresti ancora obiettare che il controllo esplicito è più pulito perché rende più ovvio e se modifichi l'implementazione, è meno probabile che tu interrompa accidentalmente il tuo contratto.
Interfacce con contratti ristretti
Se, d'altra parte, la documentazione dice
userArg must not be null.
o
If userArg is null, the behavior is undefined.
o semplicemente non dice nulla 1 , quindi chiama UpdateUser con null come userArg è una violazione del contratto e la tua implementazione può fare letteralmente qualsiasi cosa. "Tutto" ovviamente include throw in NullPointerException senza avere effetti collaterali. E farlo potrebbe essere una buona idea in termini di programmazione difensiva. Tuttavia, non vorrei ottimizzare per il caso di errore. Supponiamo che la maggior parte delle volte la tua funzione venga chiamata con argomenti validi. (Se questo non è vero, c'è qualcos'altro di cui preoccuparsi di più.) Nel raro caso in cui è chiamato fuori contratto, prima si fa un po 'di lavoro e poi fallire va bene. Soprattutto se "qualche lavoro" non ha effetti collaterali visibili.
Valori di ritorno
Il secondo controllo nel codice è molto diverso. Qui, stai verificando che userDb.Get(userArg.Id); non return null . Questo è strano.
O la documentazione di Get dice che non return null , quindi puoi fare affidamento su di esso e non devi controllare, o dice che potrebbe return null sotto alcune circostanze 2 , quindi semplicemente throw in NullPointerException probabilmente non è la reazione giusta.
Conclusione
Vorrei mantenere quei controlli per i parametri in ogni funzione che ha un contratto ampio, anche se potrebbe non essere strettamente necessario, solo perché rende il codice più ovvio.
Nelle funzioni con un contratto ristretto, utilizzerei null con parsimonia quando l'overhead delle prestazioni sembra accettabile e la funzione farebbe cose potenzialmente dannose (effetti collaterali) prima di fallire comunque. Probabilmente preferirei usare le dichiarazioni assert in questo caso, comunque. Ciò rende più chiaro che stai effettuando un controllo difensivo che verrà attivato solo in caso di violazione del contratto.
Mi sbarazzerei di tutti i controlli per i valori di return .
1 Se non si dice nulla su un parametro implica che non deve essere null potrebbe essere aperto al dibattito. Poiché null è raramente un valore valido, preferisco dichiarare esplicitamente dove è consentito e altrimenti rimanere in silenzio. Però, dovresti consultare le convenzioni del tuo progetto su questo.
2 Ad esempio, potrebbe utilizzare return value null per indicare che la voce richiesta non è stata trovata nel database. Questa potrebbe non essere la migliore scelta di design, ma tale codice esiste nella realtà.