L'operatore add della classe set restituisce un valore booleano che è vero se l'elemento (che deve essere aggiunto) non era già lì e falso altrimenti. Sta scrivendo
if (set.add(entry)) {
//do some more stuff
}
considerato un buon stile in termini di scrittura di codice pulito? Mi chiedo poiché fai due cose contemporaneamente. 1) aggiungendo l'elemento e 2) controllando se l'elemento esiste.
Sì, lo è.
Il solito punto in cui un'operazione restituisce un valore booleano è che puoi usarlo per prendere decisioni, cioè all'interno di un costrutto if . L'unico altro modo in cui potresti realizzare questo potenziale sarebbe quello di memorizzare il valore di ritorno in una variabile e poi riutilizzarlo immediatamente in un if , che è semplicemente sciocco e certamente non in alcun modo preferibile alla scrittura di if(operation()) { ... } . Quindi, continua e fallo, non ti giudicheremo (per quello).
Direi che non è il più pulito possibile, perché obbliga il manutentore a conoscere già oa cercare cosa significhi il valore restituito. Significa che il valore esisteva già, non esisteva già, è stato inserito con successo? Se non lo usi molto, non lo saprai, e anche se lo fai, è molto più carico mentale.
Preferirei quanto segue:
boolean added = set.add(entry);
if (added) {
//do some more stuff
}
Sì, un po 'più prolisso, ma il compilatore dovrebbe generare praticamente lo stesso bytecode, e anche le persone che non hanno utilizzato i set Java in anni possono seguire la logica senza cercare nulla.
Se true significa successo, allora è buono, codice chiaro.
Esiste una convenzione diffusa secondo cui una funzione o un metodo restituiscono true (o qualcosa che si considera true) in caso di successo. Finché il tuo codice lo segue, penso che mettere il metodo nel condizionale va bene.
Il codice come questo è inutilmente ingombrante a mio avviso:
boolean frobulate_succeeded = thing.frobulate();
if (frobulate_succeeded) {
...
}
Sembra che tu stia ripetendo te stesso.
Tuttavia, la domanda è ambigua sul significato del valore di ritorno. Dici "un booleano che indica se l'elemento aggiunto esisteva già", il che potrebbe implicare che vero significa che l'elemento esisteva (e che l'aggiunta non si verificava). In tal caso, cambierei idealmente il comportamento di ritorno del metodo per essere più convenzionale. Se ciò non è possibile, aggiungerei una variabile intermedia extra che ti consenta di etichettare chiaramente il risultato di ritorno nel tuo codice (come suggerito da altri).
Direi che è molto simile a C. La maggior parte delle volte preferirei avere una variabile con nome descrittivo per un risultato di mutazione e nessuna mutazione che si verifica in una condizione if .
Un compilatore eliminerà questa variabile se viene immediatamente riutilizzata. Un umano avrà più tempo a leggere la fonte; per me, è più importante.
Se qualcuno dovesse estendere la condizione aggiungendo una clausola and / or alla condizione if, potrebbero finire per non chiamare .add() in alcuni casi a causa di una valutazione di cortocircuito. A meno che non si preveda specificamente il cortocircuito, questo potrebbe finire come un errore.
Il tuo codice sembra interrompere la Separazione delle query di comando . Questo è discusso nel libro Codice pulito e Struttura delle funzioni video. Quindi, dal punto di vista del codice pulito, penso che non sia considerato di buon gusto.
“Clean code is simple and direct. Clean code reads like well-written prose. Clean code never obscures the designer’s intent but rather is full of crisp abstractions and straightforward lines of control.
-- Grady Booch author of Object Oriented Analysis and Design with Applications”
Per me l'intento del tuo codice non è chiaro. È se entrambi vengono eseguiti quando la voce viene aggiunta correttamente o anche quando esiste già? Cosa restituisce add() ? l'oggetto? Il codice di errore?
Leggi altre domande sui tag coding-style clean-code java