Istanza ripetuta della stessa classe in un metodo

Preferisci la versione A a meno che tu non abbia una ragione concreta per preoccuparti dell'effetto di molte invocazioni del metodo.

Esiste un principio generale "le variabili dovrebbero essere dichiarate il più vicino possibile al luogo in cui sono utilizzate", che dovrebbe essere seguito a meno che non si abbia una buona ragione per non farlo. La versione A rispetta questo principio e ha diversi vantaggi:

• Puoi vedere la dichiarazione dell'oggetto esattamente dove la stai usando. Nella versione B, una volta superato un semplice esempio di codice reale, la dichiarazione e l'istanza dell'oggetto potrebbero essere lontane dal suo utilizzo. Questo rende il codice meno chiaro.
• Limitare le possibilità di errore dei limiti di ambito. Nella versione A, l'oggetto esiste solo quando lo si utilizza. Nella versione B, è sospeso e accessibile ad altri metodi. È possibile che questo possa portare a un bug. Ad esempio, se devi dichiarare diversi oggetti come questo per diversi metodi, potresti accidentalmente usare quello sbagliato da qualche parte.
• La memoria viene trattenuta solo mentre si utilizza l'oggetto. La versione A può causare più allocazioni di memoria e può essere dannosa nel contesto delle lingue con garbage collection simile a Java. Ma vale la pena notare che la versione A in realtà lega meno la memoria totale a più lungo termine, perché l'oggetto esiste solo mentre è effettivamente necessario (e non del tutto se tale metodo non viene chiamato). Non è vero che una versione è decisamente preferibile in termini di memoria.

Per questo motivo preferirei A (senza sapere nient'altro su come viene usato questo codice).

È vero che B è utile in casi specifici. Come nota il tuo amico, se il metodo viene richiamato più volte, la versione B userebbe la memoria in modo più efficace (e eviterebbe ripetutamente di incorrere nel sovraccarico di instaziare l'oggetto).

Ma preoccuparsi di questo quando non si sa veramente di avere un problema di memoria o di prestazioni è un classico caso di ottimizzazione prematura. E ci sono casi in cui la versione A funziona meglio. Ad esempio, cosa succede se il tuo codice prevede la creazione di migliaia di oggetti MyClass , ma MethodY viene usato raramente? In questo caso, A potrebbe essere il design migliore in termini di memoria.

Supponendo che GeometrySplitter non abbia nessuno stato, allora la Versione A è l'opzione più pulita, più facile da leggere, poiché riduce al minimo la vita dell'oggetto a quando è effettivamente necessaria.

Tuttavia, tu dici che questa funzione è chiamata 4000 volte al secondo, e il tuo collega è preoccupato per le prestazioni. Quindi analizza quella prestazione. Configura un'imbracatura di prova ed esegui ciascuna versione a turno con 4000 colpi al secondo in un arco di tempo decente. Esamina l'utilizzo della memoria e misura esattamente come risponde.

Senza sapere quanto tempo ci vuole per creare un GeometrySplitter, è difficile sapere se creare 4000 di questi al secondo farà tassare la JVM.

L'ambito membro non risolve completamente il problema

Anche se dichiari GeometrySplitter come membro della classe, verrà comunque istanziato per istanza di MyClass , e inoltre, dovrà essere istanziato separatamente per qualsiasi altra classe che lo utilizza. Pertanto, se sei preoccupato per il sovraccarico dovuto alla costruzione di un GeometrySplitter , spostarlo fuori dall'ambito locale non risolve completamente il problema.

Usa IoC per aggirare tutto questo

In IoC , la creazione dell'oggetto è considerata una preoccupazione separata e fare in modo che MyClass si preoccupi di come istanziare qualcosa è una violazione di SRP . Questi problemi non dovrebbero essere risolti da MyClass .

Se utilizzi un contenitore IoC, elimina il problema e potresti anche risparmiare un sovraccarico di istanziazione non solo tra chiamate diverse a MyClass::methodY ma anche tra chiamate diverse a qualsiasi metodo nella classe qualsiasi che usa lo splitter.

Ad esempio:

public MyClass {

    protected readonly IUnityContainer _container;

    public MyClass(IUnityContainer container) { 
        _container = container; 
    }

    public void methodY(int i, object o) {
        IGeometrySplitter splitter = _container.Resolve<IGeometrySplitter>();
        splitter.chop();
    }
}

Le regole di istanza appartengono alla root di composizione

Se vuoi ogni volta una nuova istanza, imposta composizione root in questo modo:

container.RegisterType<IGeometrySplitter, GeometrySplitter>();

Se si desidera riutilizzare una singola istanza (non protetta da thread):

container.RegisterType<IGeometrySplitter, GeometrySplitter>(new PerThreadLifetimeManager());

Se vuoi una singola istanza (thread-safe) che viene riutilizzata:

container.RegisterType<IGeometrySplitter, GeometrySplitter>(new ContainerControlledLifetimeManager());

Quindi, registra MyClass nel contenitore, in modo che si inietti nel processo di creazione dell'istanza:

container.RegisterType<IUnityContainer>(container);
container.RegisterType<MyClass>();

Quando lo fai in questo modo, Unity si autoiniezione automaticamente come argomento del costruttore su MyClass in modo che methodY possa chiamarlo.

Per istanziare MyClass , usa:

var myClass = container.Resolve<MyClass>();

Note

• Il mio esempio sopra utilizza Unity , che è la tecnologia ac #. In Java credo che usereste Spring invece (non piuttosto sicuro). Ma il principio è indipendente dal linguaggio e le tecniche per controllare la durata degli oggetti dovrebbero essere simili.

• Sebbene questo modello non sia troppo raro, alcuni direbbero che è un anti -pattern. Dicevano che dovresti iniettare uno specifico GeometrySplitterFactory invece del contenitore IoC stesso e implementare le regole di istanza in fabbrica. Ma il principio è lo stesso: prendi le regole di istanziazione da MyClass .

Non esiste una regola generale a questo. Dipende dal caso d'uso qui.

Se il metodo viene utilizzato raramente, non vi è alcun motivo per spostare la classe GeometrySplitter al di fuori dell'ambito di MethodY .

Tuttavia, l'allocazione della memoria è un'operazione piuttosto costosa e dovrebbe essere utilizzata con cautela. Se la classe GeometrySplitter non ha proprietà che dipendono dai parametri di input di MethodY , e non assomiglia a questo, allora il secondo approccio è migliore se MethodY viene chiamato spesso. Non perdi tempo per allocare memoria, non frammentare la memoria e non sovraccaricare il garbage collector in seguito per rilasciare tutti i riferimenti non necessari.

La terza opzione sarebbe passare un'istanza di quella classe, istanziata altrove, come argomento di MethodY . In questo modo, ottieni il meglio da entrambi i mondi. Crea un'istanza di GeometrySplitter una volta e usala semplicemente dove è necessario. Questo approccio denota chiaramente anche la dipendenza di MyClass.MethodY su GeometrySplitter class, che non è chiara negli approcci precedenti senza guardare al corpo del metodo.

Supponendo che chop non abbia effetto sullo stato dell'istanza GeometrySplitter , allora A è preferibile a B, ma ancora meglio cambierebbe chop con un metodo statico.

Questo porta a un codice che è più semplice di entrambe le opzioni:

public MyClass {
    public void methodX() {}

    public void methodY(int i, object o) {
        GeometrySplitter.chop(i, o);
    }

    public void methodZ() {}
}

(Vorrei assumere chop non influenza lo stato di GeometrySplitter perché altrimenti le due opzioni che presentate sarebbero funzionalmente diverse e una di esse non funzionerebbe correttamente.)

Più in generale, si desidera sempre ridurre la quantità di stato mutabile che può influenzare qualsiasi sezione di codice. A parità di condizioni, vogliamo limitare lo stato mutabile al più stretto possibile . Ciò significa che preferiamo metodi statici senza effetti collaterali ai metodi di istanza e, se ciò non è possibile, preferiamo le istanze con l'ambito di un singolo metodo (opzione A) alle istanze con ambito di classe (opzione B).

Creare nuovi oggetti più e più volte richiede tempo. Il cambiamento per evitare questo è banale. Così misurate quanto tempo sarete al sicuro, e quando è significativo, fate il cambiamento banale. Se non misurate, ovviamente il guadagno non è significativo.

E un'altra cosa: se la tua applicazione è multi-thread, è meglio essere assolutamente sicuri che il tuo "splitter" sia thread-safe, cioè che funzioni correttamente se lo stesso oggetto splitter viene utilizzato da più thread, altrimenti tu stanno creando una bomba a tempo per te stesso.

Hai ragione e (potenzialmente) torto.

Hai ragione se il business case dice davvero che non devi chiamare il metodo 4000 volte. Allora va bene.

Il tuo collega ha ragione dato che è davvero una pratica di codice migliore, non solo per i motivi menzionati dal tuo collega, ma perché questo oggetto è disponibile in tutta MyClass. Questo promuove classi più piccole e più specifiche (come MyClass) e incapsulamento (nasconde l'implementazione e il funzionamento di GeometrySplitter al mondo esterno).

Forse oggi hai bisogno di GeometrySplitter solo con quel metodo, ma in futuro potresti ottenere un altro business case in cui dovrai costruire un altro metodo e istanziarlo di nuovo e poi avrai più refactoring da fare. Il modo in cui promuovi la creazione dell'oggetto lo apre alle cosiddette classi "Dio" e un sacco di refactoring inutile.