Quanta logica in Getters

Il modo in cui il lavoro ti dice di fare le cose è zoppo.

Come regola generale, il modo in cui faccio le cose è il seguente: se ottenere le cose è computazionalmente economico, (o se molte probabilità sono che si troveranno nella cache), allora il tuo stile di getStuff () è bene. Se l'acquisizione di materiale è nota come dispendiosa dal punto di vista computazionale, tanto costosa da rendere necessaria l'interfaccia a livello di costi, allora non chiamerei getStuff (), lo chiamerei calculateStuff () o qualcosa del genere, in modo da indicare che ci sarà del lavoro da fare.

In entrambi i casi, il modo in cui il lavoro ti dice di fare le cose è zoppo, perché getStuff () esploderà se loadStuff () non è stato chiamato in anticipo, quindi essenzialmente vogliono complicare la tua interfaccia introducendo l'ordine di -operazioni complesse ad esso. L'ordine delle operazioni è praticamente il peggior tipo di complessità che riesco a pensare.

La logica dei getter è perfetta.

Ma ottenere dati da un database è molto più che "logico". Comporta una serie di operazioni costose molto in cui molte cose possono andare storte e in modo non deterministico. Esiterei a farlo implicitamente in un getter.

D'altra parte, la maggior parte degli ORM supporta il caricamento lento delle raccolte, che è fondamentalmente esattamente quello che stai facendo.

Penso che in base a "Clean Code" dovrebbe essere diviso il più possibile, in qualcosa del tipo:

public List<Stuff> getStuff() {
   if (hasStuff()) {
       return stuff;
   }
   loadStuff();
   return stuff;
}

private boolean hasStuff() {
    if (stuff == null) {
       return false;
    }
    if (cacheInvalid()) {
       return false;        
    }
    return true;
} 

private void loadStuff() {
    stuff = getStuffFromDatabase();
}

Ovviamente, questa è una totale assurdità, dato che la bella forma, che hai scritto, fa la cosa giusta con una frazione di codice che chiunque comprende a colpo d'occhio:

public List<Stuff> getStuff() {
   if (stuff == null || cacheInvalid()) {
       stuff = getStuffFromDatabase();
   }
   return stuff;
}

Non dovrebbe essere il mal di testa del chiamante come la roba è sotto il cofano, e in particolare non dovrebbe essere il mal di testa del chiamante ricordare di chiamare le cose in qualche "ordine giusto" arbitrario.

They tell me there should be as little logic as possible in getters and setters.

Ci deve essere tutta la logica necessaria per soddisfare i bisogni della classe. La mia preferenza personale è il meno possibile, ma quando si mantiene il codice, di solito si deve lasciare l'interfaccia originale con i getter / setter esistenti, ma mettere molta logica in essi per correggere la nuova logica di business (ad esempio, un "cliente" "getter in un ambiente post-911 deve incontrare" conosci il tuo cliente "e OFAC regolamenti, combinati con una politica aziendale che vieta l'apparizione di clienti provenienti da determinati paesi dall'apparire [come Cuba o Iran]).

Nel tuo esempio, preferisco il tuo e non mi piace il campione "zio bob" poiché la versione "zio bob" richiede che gli utenti / manutentori ricordino di chiamare loadStuff() prima di chiamare getStuff() - questa è una ricetta per il disastro se nessuno dei tuoi manutentori dimentica (o peggio, non lo sa mai). La maggior parte dei luoghi in cui ho lavorato negli ultimi dieci anni utilizza ancora un codice che ha più di un decennio, quindi la facilità di manutenzione è un fattore critico da considerare.

Hai ragione, i tuoi colleghi hanno torto.

Dimentica le regole empiriche di tutti su cosa un metodo dovrebbe o non dovrebbe fare. Una classe dovrebbe presentare un'astrazione di qualcosa. La tua classe ha un valore leggibile di stuff . In Java è normale usare i metodi 'get' per leggere le proprietà. Sono stati scritti miliardi di linee di framework che prevedono di leggere stuff chiamando getStuff . Se assegni alla tua funzione fetchStuff o qualsiasi altra cosa diversa da getStuff , la tua classe sarà incompatibile con tutti questi framework.

Potresti puntarli su Hibernate, dove 'getStuff ()' può fare cose molto complicate e lancia una RuntimeException in caso di fallimento.

Sembra che questo potrebbe essere un dibattito purista contro un'applicazione che potrebbe essere influenzato da come preferisci controllare i nomi delle funzioni. Dal punto di vista applicato, preferirei piuttosto vedere:

List<String> names = clientRoster.getNames();
List<String> emails = clientRoster.getEmails();

Al contrario di:

myObject.load();
List<String> names = clientRoster.getNames();
List<String> emails = clientRoster.getEmails();

O ancora peggio:

myObject.loadNames();
List<String> names = clientRoster.getNames();
myOjbect.loadEmails();
List<String> emails = clientRoster.getEmails();

Il che tende a rendere l'altro codice molto più ridondante e più difficile da leggere perché devi iniziare a esaminare tutte le chiamate simili. Inoltre, chiamare le funzioni del caricatore o simili interrompe l'intero scopo di utilizzare anche OOP in quanto non si viene più astratti dai dettagli dell'implementazione dell'oggetto con cui si sta lavorando. Se hai un oggetto clientRoster , non dovresti preoccuparti di come getNames funzioni, come faresti se dovessi chiamare un loadNames , dovresti solo sapere che getNames ti dà un List<String> con i nomi dei clienti.

Quindi, sembra che il problema riguardi più la semantica e il miglior nome per la funzione per ottenere i dati. Se la società (e altri) ha un problema con il prefisso get e set , allora che ne dici di chiamare la funzione qualcosa come retrieveNames invece? Dice cosa sta succedendo, ma non implica che l'operazione sia istantanea come ci si aspetterebbe da un metodo get .

In termini di logica in un metodo accessor, mantenerlo al minimo in quanto sono generalmente implicati per essere istantanei con solo l'interazione nominale che si verifica con la variabile. Tuttavia, anche questo generalmente si applica solo a tipi semplici, tipi di dati complessi (ad esempio List ) trovo più difficile incapsulare correttamente in una proprietà e in genere uso altri metodi per interagire con essi in contrasto con un mutatore e un accessorio rigoroso.

Chiamare un getter dovrebbe mostrare lo stesso comportamento della lettura di un campo:

• Dovrebbe essere economico recuperare il valore
• Se si imposta un valore con il setter e quindi lo si legge con il getter, il valore dovrebbe essere lo stesso
• Ottenere il valore non dovrebbe avere effetti collaterali
• Non dovrebbe generare un'eccezione

Un getter che richiama altre proprietà e metodi per calcolare il proprio valore implica anche una dipendenza. Ad esempio, se la proprietà deve essere in grado di calcolare se stessa, e così facendo richiede l'impostazione di un altro membro, è necessario preoccuparsi di riferimenti nulli accidentali se si accede alla proprietà nel codice di inizializzazione in cui tutti i membri non sono necessariamente impostati. / p>

Questo non significa 'non accedere mai ad un altro membro che non è il campo di supporto delle proprietà all'interno del getter', significa solo prestare attenzione a ciò che stai implicando riguardo allo stato richiesto dell'oggetto, e se questo corrisponde al contesto in cui ci si aspetta che questa proprietà sia accessibile.

Tuttavia, nei due esempi concreti che hai dato, la ragione per cui scegliere uno sull'altro è completamente diversa. Il tuo getter è inizializzato al primo accesso, ad es. Inizializzazione pigra . Si presume che il secondo esempio sia inizializzato in qualche punto precedente, ad esempio Inizializzazione esplicita .

Quando si verifica esattamente l'inizializzazione può o non può essere importante.

Ad esempio potrebbe essere molto molto lento e deve essere fatto durante una fase di caricamento in cui l'utente si aspetta un ritardo, piuttosto che una prestazione inaspettata quando l'utente innesca per primo l'accesso (es. clic destro dell'utente, menu di scelta rapida appare, l'utente ha già fatto clic di nuovo con il tasto destro del mouse).

Inoltre, a volte c'è un punto ovvio nell'esecuzione in cui si verifica tutto ciò che può influenzare / sporcare il valore della proprietà memorizzata nella cache. Potresti anche verificare che nessuna delle dipendenze cambi e generare eccezioni in seguito. In questa situazione ha senso memorizzare anche il valore in quel punto, anche se non è particolarmente costoso da calcolare, solo per evitare di rendere l'esecuzione del codice più complessa e più difficile da seguire mentalmente.

Detto questo, Lazy Initialization ha molto senso in molte altre situazioni. Quindi, come spesso accade nella programmazione, è difficile ridurlo a una regola, si scende al codice concreto.

Fallo come @MikeNakis ha detto ... Se hai appena preso la roba allora va bene ... Se fai qualcos'altro, crea una nuova funzione che fa il lavoro e rendilo pubblico.

Se la tua proprietà / funzione sta facendo solo quello che dice il nome, allora non c'è molto spazio per la complicazione rimasta. La coesione è la chiave IMO

Un getter dovrebbe essere un getter. Quindi meno logica potrebbe essere più adatta. Se pensi che in generale sia un modo per accedere a un campo o sth. altro. Manupulation and validation, ecc. Sono lavori diversi. Il mio è solo un'idea per mantenerli semplici.

Personalmente, esporrò il requisito di Stuff tramite un parametro nel costruttore e consentirò a qualsiasi classe di creare un'istanza per fare il lavoro di capire da dove dovrebbe provenire. Se roba è nulla, dovrebbe restituire null. Preferisco non tentare soluzioni intelligenti come l'originale dell'Op perché è un modo semplice per nascondere i bug in profondità nella tua implementazione, dove non è del tutto ovvio cosa potrebbe andare storto quando qualcosa si rompe.

Ci sono problemi più importanti quindi solo "appropriatezza" qui e dovresti basare la tua decisione su quelli . Principalmente, la grande decisione qui è se vuoi consentire alle persone di bypassare la cache o meno.

1. Innanzitutto, pensa se esiste un modo per riorganizzare il codice in modo che tutte le chiamate di carico necessarie e la gestione della cache vengano eseguite nel costruttore / inizializzatore. Se questo è possibile, puoi creare una classe il cui invariant ti permette di fare al getter semplice dalla parte 2 con la sicurezza del getter complesso dalla parte 1. (Uno scenario win-win)

2. Se non riesci a creare una classe del genere, decidi se hai un compromesso e devi decidere se vuoi consentire al consumatore di saltare il codice di controllo della cache o meno.

   1. Se è importante che il consumatore non salti mai il controllo della cache e non ti dispiaccia le penalità relative alle prestazioni, allora accoppi il controllo all'interno del getter e rendi impossibile al consumatore fare la cosa sbagliata.

   2. Se è corretto saltare il controllo della cache o è molto importante garantire prestazioni O (1) nel getter, quindi utilizzare chiamate separate.

Come forse già notato, non sono un grande fan della filosofia "clean-code", "split tutto in piccole funzioni". Se hai un sacco di funzioni ortogonali che possono essere chiamate in qualsiasi ordine, suddividendole otterrai più potenza espressiva a costi ridotti. Tuttavia, se le tue funzioni hanno dipendenze degli ordini (o sono davvero utili solo in un particolare ordine), la loro divisione aumenta solo il numero di modi in cui puoi fare cose sbagliate, aggiungendo poco beneficio.

Secondo me, Getters non dovrebbe avere molta logica in loro. Non dovrebbero avere effetti collaterali e non si dovrebbe mai ottenere un'eccezione da loro. A meno che, naturalmente, tu sappia cosa stai facendo. La maggior parte dei miei getter non ha alcuna logica e basta andare in un campo. Ma la notevole eccezione è stata con una API pubblica che doveva essere il più semplice possibile da usare. Così ho avuto un getter che fallirebbe se non fosse stato chiamato un altro getter. La soluzione? Una riga di codice come var throwaway=MyGetter; nel getter che dipendeva da esso. Non ne sono orgoglioso, ma continuo a non vedere un modo più pulito per farlo

Sembra una lettura dalla cache con caricamento lazy. Come altri hanno notato, il controllo e il caricamento possono appartenere ad altri metodi. Potrebbe essere necessario caricare il caricamento in modo che non vengano caricati venti thread contemporaneamente.

Potrebbe essere appropriato usare un nome getCachedStuff() per il getter in quanto non avrà un tempo di esecuzione coerente.

A seconda di come funziona la routine cacheInvalid() , il controllo di null potrebbe non essere necessario. Non mi aspetto che la cache sia valida a meno che stuff non sia stato popolato dal database.

IMHO è molto semplice se usi un design per contratto. Decidi cosa deve fornire il tuo getter e basta codificarti di conseguenza (codice semplice o una logica complessa che può essere coinvolta o delegata da qualche parte).