Nome per questo antipattern? Campi come variabili locali [chiuso]

This is obviously bad, right?

Sì.

• Rende i metodi non rientranti, il che è un problema se vengono chiamati sulla stessa istanza in modo ricorsivo o in un contesto multi-thread.

• Significa che lo stato da una chiamata passa a un'altra chiamata (se si dimentica di reinizializzare).

• Rende il codice difficile da capire perché devi verificare quanto sopra per essere sicuro di ciò che il codice sta effettivamente facendo. (Contrasto con l'utilizzo di variabili locali).

• Rende ogni istanza Foo più grande di quanto deve essere. (E immagina di farlo per le variabili N ...)

Is there a name for this antipattern?

IMO, questo non merita di essere chiamato antipattern. È solo una cattiva abitudine di codifica / un uso improprio dei costrutti Java / codice crap. È il tipo di cosa che potresti vedere nel codice di uno studente quando lo studente ha saltato le lezioni e / o non ha attitudine per la programmazione. Se lo vedi nel codice di produzione, è un segno che devi fare molte più revisioni del codice ...

Per la cronologia, il modo corretto per scrivere questo è:

public class Foo
{
    public methodA()
    {
        Bar bar = new Bar();  // Use a >>local<< variable!!
        bar.a();
    }

    // Or more concisely (in this case) ...
    public methodB()
    {
        new Bar().b();
    }
}

Si noti che ho anche corretto il metodo e i nomi delle variabili per conformarsi alle regole di stile accettate per gli identificatori Java.

Lo definirei grandangolo non necessario per una variabile. Questa impostazione consente anche condizioni di gara se più thread accedono a MethodA e MethodB.

Questo è un caso specifico di un modello generale noto come global doorknobbing . Quando si costruisce una casa, si è tentati di acquistare solo una maniglia e lasciarla in giro. Quando qualcuno vuole usare una porta o un armadio, si limita ad afferrare quella maniglia globale. Se le maniglie delle porte sono costose, può essere un buon progetto.

Sfortunatamente, quando il tuo amico arriva e la maniglia della porta viene usata simultaneamente in due posti diversi, la realtà si infrange. Se la maniglia della porta è così costosa che puoi permetterti solo una, allora vale la pena costruire un sistema che permetta alle persone di aspettare il proprio turno per la maniglia della porta.

In questo caso la maniglia è solo un riferimento, quindi è economico. Se la maniglia era un oggetto reale (e stavano riutilizzando l'oggetto invece del solo riferimento), allora potrebbe essere abbastanza costoso da essere un prudent global doorknob . Tuttavia, quando è a buon mercato, è noto come cheapskate global doorknob .

Il tuo esempio è della varietà cheapskate .

La preoccupazione principale qui sarebbe la concorrenza - Se Foo viene utilizzato da più thread, hai un problema.

Oltre a ciò, è semplicemente sciocco - le variabili locali gestiscono perfettamente i loro cicli di vita. Sostituirli con variabili di istanza che devono essere annullate quando non sono più utili è un invito all'errore.

È un caso specifico di "scoping improprio", con un lato di "riutilizzo variabile".

Non è un anti-modello. Gli anti-pattern hanno alcune proprietà che lo fanno sembrare una buona idea, che porta le persone a farlo apposta; sono pianificati come modelli e poi vanno terribilmente storto.

È anche ciò che rende dibattiti se qualcosa è un pattern, un anti-pattern o un pattern comunemente applicato male che ha ancora usi in alcuni punti.

Questo è solo sbagliato.

Per aggiungere altro.

Questo codice è superstizioso, o nel migliore dei casi una pratica di culto del carico.

Una superstizione è qualcosa fatto senza una chiara giustificazione. Potrebbe essere correlato a qualcosa di reale, ma la connessione non è logica.

Una pratica di culto del carico è quella in cui cerchi di copiare qualcosa che hai appreso da una fonte più esperta, ma in effetti stai copiando gli artefatti di superficie piuttosto che il processo (così chiamato per un culto in Papua Nuova Guinea chi farebbe le radio di controllo degli aerei di bambù sperando di far tornare gli aerei giapponesi e americani della seconda guerra mondiale.

In entrambi i casi non c'è alcun caso reale da fare.

Un anti-pattern è un tentativo di miglioramento ragionevole, sia nel piccolo (quella ramificazione extra per affrontare quel caso extra che deve essere affrontato, che porta al codice spaghetti) o nel grande dove tu deliberatamente implementare un modello che sia discreditato o discusso (molti descrivono i singleton come tali, con alcuni che escludono solo la scrittura - ad esempio oggetti di logging o di sola lettura, ad esempio oggetti di configurazione - e alcuni potrebbero condannare anche quelli) o altrimenti dove si ' risolvendo il problema sbagliato (quando .NET è stato introdotto per la prima volta, MS ha consigliato un pattern per gestire lo smaltimento quando si avevano sia campi non gestiti sia campi gestiti usa e getta - in effetti gestisce bene quella situazione, ma il vero problema è che tu hai entrambi i tipi di campo nella stessa classe)

In quanto tale, un anti-modello è qualcosa che una persona intelligente che conosce bene la lingua, il dominio problematico e le librerie disponibili deliberatamente, ha ancora (o si sostiene che abbia) uno svantaggio che travolge il rialzo.

Dato che nessuno di noi inizia a conoscere bene una determinata lingua, il dominio dei problemi e le librerie disponibili, e poiché tutti possono perdere qualcosa passando da una soluzione ragionevole a un'altra (ad esempio, inizia a memorizzare qualcosa in un campo per un buon uso, e poi prova a rifattarlo via ma non completare il lavoro, e finirai con il codice come nella domanda), e dal momento che tutti noi ci perdiamo di tanto in tanto nell'apprendimento, tutti abbiamo creato un codice superstizioso o di culto del carico ad un certo punto. La cosa buona è che sono in realtà più chiari per identificare e correggere rispetto agli anti-schemi. I veri anti-schemi sono probabilmente non anti-pattern, o hanno una qualità attraente, o almeno hanno un certo modo di attirarli in uno anche se identificati come cattivi (troppi e troppo pochi livelli sono entrambi controvogliamente cattivi, ma evitando uno conduce all'altro).

(1) Direi che non va bene.

Sta eseguendo il mantenimento manuale che lo stack può eseguire automaticamente con variabili locali.

Non vi è alcun vantaggio in termini di prestazioni poiché "nuovo" viene chiamato ogni richiamo del metodo.

EDIT: il footprint della memoria della classe è maggiore perché il puntatore della barra occupa sempre la memoria per tutta la durata della classe. Se il puntatore della barra era locale, userebbe solo la memoria per tutta la durata della chiamata al metodo. La memoria del puntatore è solo una goccia nell'oceano, ma è ancora una goccia inutile.

La barra è visibile ad altri metodi nella classe. I metodi che non usano la barra non dovrebbero essere in grado di vederlo.

Errori basati sullo stato. In questo caso particolare gli errori di base dello stato dovrebbero verificarsi solo in multi-threading poiché "new" viene chiamato ogni volta.

(2) Non so se c'è un nome per questo dato che in realtà sono diversi problemi, non uno.
- servizio di pulizia manuale quando è disponibile l'opzione automatica
- stato necessario | - stato che vive più a lungo che è la vita
--visibilità o ambito è troppo alto

Lo chiamerei "Wandering Xenophobe". Vuole essere lasciato solo ma finisce senza sapere bene dove o cosa sia. Quindi è una brutta cosa come altri hanno affermato.

Andando controcorrente qui, ma mentre non considero l'esempio dato come uno stile di codifica buono come nella sua forma attuale, lo modello esiste mentre si effettua il test delle unità.

Questo metodo abbastanza standard per eseguire test delle unità

public class BarTester
{
    private Bar bar;

    public Setup() { bar = new Bar(); }
    public Teardown() { bar = null; }

    public TestMethodA()
    {
        bar.A();        
    }

    public TestMethodB()
    {
        bar.B();
    }
}

è solo un refactoring di questo equivalente del codice OP

public class BarTester
{
    private Bar bar;

    public TestMethodA()
    {
        bar = new Bar();
        bar.A();
        bar = null;
    }

    public TestMethodB()
    {
        bar = new Bar();
        bar.B();
        bar = null;
    }
}

Non scriverei mai il codice come indicato dall'esempio di OP, urla il refactoring, ma considero il modello non essere né non valido né un antipattern .

Questo odore di codice è indicato come campo temporaneo nel Refactoring di Beck / Fowler.

link

Modificato per aggiungere ulteriori spiegazioni su richiesta dei moderatori: Puoi leggere ulteriori informazioni sull'odore del codice nell'URL di riferimento sopra o nella copia cartacea del libro. Dal momento che l'OP cercava il nome per questo particolare odore di antipattern / codice, ho pensato che citare un riferimento finora non menzionato ad esso da una fonte consolidata che ha più di 10 anni sarebbe utile, anche se mi rendo perfettamente conto che sono in ritardo gioco su questa domanda, quindi è improbabile che la risposta venga votata o accettata.

Se non si tratta di una promozione troppo personale, faccio riferimento anche a questo particolare odore di codice nel mio prossimo corso Pluralsight, Refactoring Fundamentals, che mi aspetto sarà pubblicato nell'agosto 2013.

Per aggiungere più valore, parliamo di come risolvere questo particolare problema. Ci sono diverse opzioni Se il campo è realmente utilizzato da un solo metodo, può essere abbassato a una variabile locale. Tuttavia, se più metodi utilizzano il campo per comunicare (al posto dei parametri), questo è il caso classico descritto in Refactoring e può essere corretto sostituendo il campo con i parametri o se i metodi che funzionano con questo codice sono strettamente correlati related, Extract Class può essere usato per estrarre i metodi e i campi nella loro classe, in cui il campo è sempre in uno stato valido (forse impostato durante la costruzione) e rappresenta lo stato di questa nuova classe. Se si sta percorrendo il percorso dei parametri, ma ci sono troppi valori da aggirare, un'altra opzione è introdurre un nuovo Oggetto Parametro, che può essere passato al posto di tutte le singole variabili.

Direi che quando si arriva fino a questo punto, questa è davvero una mancanza di coesione, e potrei dargli il nome "False Cohesion". Coinvolgerei (o se lo ricaverevo da qualche parte e dimenticarlo, "prendere in prestito") questo termine perché la classe sembra essere coesiva in quanto i suoi metodi sembrano operare su un campo membro. Tuttavia, in realtà non lo fanno, cioè l'apparente coesione è in realtà falsa.

Per quanto riguarda se è cattivo o no, direi che lo è chiaramente, e penso che Stephen C faccia un ottimo lavoro nel spiegarne il perché. Tuttavia, se meriti di essere chiamato "anti-pattern" o no, penso che questo e qualsiasi altro paradigma di codificazione potrebbe fare con un'etichetta, dal momento che generalmente rende più semplice la comunicazione.

A parte i problemi già menzionati, l'utilizzo di questo approccio in ambiente senza la garbage collection automatica (ad esempio C ++) porterebbe a perdite di memoria, poiché gli oggetti temporanei non vengono liberati dopo l'uso. (Anche se questo può sembrare un po 'esagerato, ci sono persone là fuori che cambierebbero semplicemente "null" in "NULL" e sii felice che il codice venga compilato.)

Questo mi sembra un'erronea applicazione errata del modello Flyweight. Purtroppo conosco alcuni sviluppatori che devono utilizzare la stessa "cosa" più volte con metodi diversi e semplicemente estrapolarli in un campo privato in un tentativo errato di risparmiare memoria o migliorare le prestazioni o qualche altra pseudo-ottimizzazione bizzarra. Nella loro mente stanno cercando di risolvere un problema simile visto che Flyweight intende rispondere solo a come lo stanno facendo in una situazione in cui non è in realtà un problema che deve essere risolto.

L'ho incontrato molte volte, di solito quando rinfresca il codice precedentemente scritto da qualcuno che ha scelto VBA For Dummies come primo titolo e ha continuato a scrivere un sistema relativamente grande in qualunque lingua fossero incappati.

Dire che è una pessima pratica di programmazione è giusto, ma potrebbe essere stato solo il risultato di non avere internet e amp; risorse peer reviewed come quelle di cui godiamo oggi che potrebbero aver guidato lo sviluppatore originale lungo un percorso "più sicuro".

Tuttavia non merita il tag "antipattern", ma è stato bello vedere i suggerimenti su come l'OP potrebbe essere ricodificato.