Il metodo dovrebbe descrivere i suoi effetti collaterali? [duplicare]

Il passaggio 1 è ovviamente fatto in modo che il tuo metodo non abbia effetti collaterali.

Ma concentriamoci sull'esempio. Sì, dovresti generalmente astrarre ciò che una funzione fa da come la fa, ma gli effetti collaterali fanno ancora parte della cosa sta facendo la funzione. Rendendoli espliciti nel nome, stai rendendo il codice più chiaro su cosa sta succedendo, portando a meno bug.

Questo tipo di funzione che funziona con i flussi ha anche un significato speciale. Il chiamante di questo tipo di funzione deve sapere se lo stream è nuovo o no poiché quel tipo di cose indica al programmatore se è necessario pulire lo stream o meno. Gestire quel tipo di proprietà delle risorse è uno scenario rischioso che porta a tutti i tipi di bug sgradevoli (specialmente nelle lingue gestite non di memoria). Ciò rende ancor più importante la chiara comunicazione delle intenzioni.

Ma fai tieni a mente che viene creato, perché può generare una IOException!

Infatti, l'unica volta che può lanciare è la prima volta che viene chiamato. Ogni volta dovrai mettere un blocco try / catch inutile (supponendo che si tratti di Java) nel codice chiamante per qualcosa che non accadrà mai.

Mi piacerebbe refactoring questo codice per avere un metodo di creazione del flusso esplicito. Avere I / O avviene in un getter non è generalmente buono se l'I / O va mai storto.

(Se la creazione dell'oggetto non implica l'I / O, sarei d'accordo con la tua valutazione)

So, for me, this method does one thing. It retrieves (gets) ObjectOutputStream object as its name says

No. Non puoi recuperare qualcosa che non esiste. Creare un oggetto che prima non esisteva e restituire quell'oggetto cambia lo stato del programma (possibilmente con altri effetti collaterali significativi). Non è un recupero. Il recupero dovrebbe essere un'operazione sicura a basso impatto.

La creazione di un nuovo oggetto e la sua restituzione modifica lo stato del programma. Per lo meno, sta aggiungendo un nuovo oggetto all'heap, consumando un po 'più di memoria e usando tutti i cicli della CPU coinvolti nella creazione di oggetti. Solo questo può avere un impatto significativo sulle prestazioni delle applicazioni se questa funzione viene chiamata frequentemente.

E la creazione di un nuovo oggetto potrebbe avere altri effetti collaterali:

• Apertura di un file
• Avvio di una discussione
• Scrittura su un database
• Apertura di un socket di rete
• Ripristino dello stato di altri oggetti

Sai quanti di quelli che Oos possono attivare alla creazione? Se tutto quello che vuoi (e tutto ciò che ti viene dato) è un riferimento all'oggetto esistente, nessuno di questi ha bisogno di infastidirti.

Il mio ultimo punto è molto importante. Il frammento di codice implica che esiste solo uno Oos in questo contesto. Quindi la creazione di un nuovo Oos può cancellare lo stato degli oggetti correlati (cache, contatori, stack) e restituirli a una condizione di verginità. Oops.

renaming it to createOrReturnOos would be revealing implementation details to the caller.

Se l'oggetto Oos non è mai stato restituito al chiamante, se fosse una cosa transitoria e nascosta, ciò sarebbe vero. Ma stai restituendo l'oggetto creato. La sua provenienza e la sua storia sono importanti.

And as a caller I just want to get ObjectOutputStream and I don't care about how was it retrieved/created.

Dovresti. Sapendo quali operazioni cambiano stato e quali no è molto importante.

• Alcune risorse (ad es. database, cache di dati in cluster) scalano avendo un nodo che prende scritture mentre tutte le altre possono essere lette solo da. Se non sai quali funzioni cambiano stato e quali no, l'applicazione si interromperà in modo imprevisto.
• Alcune risorse interne (ad esempio pool di thread) possono essere esaurite da problemi con una sola funzione / componente. Per motivi di resilienza, è buona prassi separare queste risorse in pool separati e assegnare componenti potenzialmente distruttivi ai pool separati, in modo che un componente che esaurisce improvvisamente una risorsa non muti di fame gli altri. Ma per farlo devi sapere che quali funzioni consumano queste risorse.
• I problemi di sicurezza spesso richiedono che tu sappia quali funzioni cambiano stato e quali appena lo segnalano. Se si dispone di tale conoscenza, le funzioni che modificano lo stato possono essere limitate in modo che solo i processi privilegiati possano utilizzarle.
• Il degrado aggraziato (una funzionalità desiderabile) significa che le tue applicazioni possono ancora fare molto anche se hanno perso parte delle loro risorse (ad es. il database principale, dove possono essere eseguite le scritture). Un'applicazione ben progettata può ancora restituire informazioni anche se non può cambiarle. Ma la progettazione di questo codice richiede di sapere quali funzioni cambiano stato.

Finora, ho solo parlato di effetti collaterali che potresti non aver considerato e perché sono importanti. Ma c'è un'altra considerazione significativa.

Disonestà .

Stai mentendo al chiamante.

Il chiamante dovrebbe aspettarsi ObjectOutputStream per restituire il flusso esistente. Può avere uno stato significativo (una cache, una cronologia delle operazioni, output scritto su un file) che il chiamante si aspetta di ereditare, anche se non hanno accesso diretto. In particolare, gli stream tendono ad avere molto di stato e storia. Bene, hai mentito a loro. Ecco un oggetto nuovo di zecca senza quello stato. Loro non lo sanno. Non dovresti mentire in questo modo.

Forse là dovrebbe essere un Oos esistente nel contesto attuale. La mancanza di uno può significare che qualcosa è morto o un bug altrove nel codice ha eliminato il vecchio Oos . Ora hai nascosto quell'errore. Il debugging sarà divertente .

Inoltre stai negando al chiamante la libertà di creare il proprio flusso di controllo. Ora, io sono un tipo di FP e sospetto delle eccezioni, ma qui siamo a Java. È un idioma Java comune fare cose che possono o meno rompere e lasciare blocchi catch per gestire i casi in cui lo fanno. A volte un intero percorso di codice vive solo lì. Stai negando l'opzione al chiamante. E altre cose più semplici.

Non crea automaticamente cose che non c'erano quando non ti è stato chiesto di farlo. Se il chiamante chiede qualcosa che non è lì, diglielo. Lascia che decida cosa fare dopo.

Smettila di mentirgli.

maybe Bob found a bad example of what he wanted to say

Non l'ha fatto. Clean Code ha alcuni dei migliori scritti sullo sviluppo del software che abbia mai letto e non c'è nulla nella sezione che citi di cui mi lamenterei.

Sì, penso che il nome del metodo dovrebbe descrivere gli effetti collaterali.

Per prima cosa, questo esempio utilizza l'eccezione verificata, che di per sé rivela i dettagli dell'implementazione. L'invocazione di questo metodo richiede comunque il blocco try / catch (senza motivo, dal momento che può essere gestito all'interno e generare NPE o restituire null , il che non migliorerebbe la situazione, ma almeno renderà più pulito dal lato del chiamante), quindi un nome di metodo più chiaro non farebbe male.

Preferirei chiamarlo getSingletonOos , tuttavia, poiché è ciò che effettivamente fa. getOos suona come un semplice vecchio getter ed è molto fuorviante. Confronto:

// Plain getter, no side effect.
public ObjectOutputStream getOos() {
    return m_oos;
}

// Additional side effects are described by the name.
public ObjectOutputStream getSingletonOos() throws IOException {
    if (m_oos == null) {
        m_oos = new ObjectOutputStream(m_socket.getOutputStream());
    }
    return m_oos;
}

Il secondo esempio fa più del primo e dovrebbe riflettersi nel nome. In quale altro modo diresti la differenza?

Come altri hanno sottolineato, il tuo metodo espone effettivamente il fatto che è più di un semplice getter, poiché rende pubblico il fatto che potrebbe generare un'eccezione. Quindi, potresti anche chiarire nel nome del metodo che funziona sia come getter che come factory, altrimenti qualcuno che lo usa è disapprovato dal fatto che un getter innocente sia in grado di lanciare un fa eccezione.

In scenari alternativi che non sono complicati da eccezioni dichiarate pubblicamente, va così:

La questione se pubblicizzare gli effetti collaterali di un metodo è non può essere esaminata osservando quel singolo metodo, ma prendendo in considerazione l'oggetto nel suo complesso.

The fact that a method has side effects should be publicized if these side effects are in any way, shape, or form visible through the public interface of the object to which the method belongs.

Quindi, se il tuo oggetto riesce a nascondere completamente il fatto che alcuni dei suoi metodi eseguono internamente effetti collaterali, allora va bene non pubblicizzare questo fatto nel nome di un qualsiasi metodo pubblico. Tuttavia, se, invocando altri metodi dell'oggetto, è possibile scoprire il fatto che gli effetti collaterali hanno avuto luogo, allora quei metodi che causano questi effetti collaterali dovrebbero indicarlo nel loro nome.