Ci sono diversi problemi con questo pezzo di codice, che, a proposito, può essere abbreviato in questo modo:
public List<Money> getMoneyByPersons() {
return persons.size() == 1 ?
moneyService.getMoneyIfHasOnePerson() :
moneyService.getMoney();
}
-
Non è chiaro il motivo per cui una persona è un caso speciale. Suppongo che ci sia una specifica regola aziendale che dice che ottenere denaro da una persona è radicalmente diverso dall'ottenere denaro da più persone. Tuttavia, devo andare a dare un'occhiata sia a getMoneyIfHasOnePerson che a getMoney , sperando di capire perché ci sono casi distinti.
-
Il nome getMoneyIfHasOnePerson non sembra corretto. Dal nome, mi aspetterei che il metodo verifichi se c'è una sola persona e, se questo è il caso, ottenere denaro da lui; altrimenti, non fare nulla. Dal tuo codice, questo non è ciò che sta accadendo (o stai facendo la condizione due volte).
-
C'è qualche motivo per restituire un List<Money> piuttosto che una raccolta?
Torna alla tua domanda, perché non è chiaro il motivo per cui esiste un trattamento speciale per una persona, la prima cifra dovrebbe essere sostituita da una costante, a meno che ci sia un'altra modo per rendere esplicite le regole. Qui, uno non è molto diverso da qualsiasi altro numero magico. Potresti avere regole aziendali che dicono che il trattamento speciale si applica a una, due o tre persone o solo a più di dodici persone.
How can I distinguish context to choose the best solution?
Fai ciò che rende il tuo codice più esplicito.
Esempio 1
Immagina la seguente parte di codice:
if (sequence.size() == 0) {
return null;
}
return this.processSequence(sequence);
C'è zero qui un valore magico? Il codice è piuttosto chiaro: se non ci sono elementi nella sequenza, non elaborarlo e restituire un valore speciale. Ma questo codice può anche essere riscritto in questo modo:
if (sequence.isEmpty()) {
return null;
}
return this.processSequence(sequence);
Qui, non più costante, e il codice è ancora più chiaro.
Esempio 2
prendi un altro pezzo di codice:
const result = Math.round(input * 1000) / 1000;
Non ci vuole molto tempo per capire cosa fa in linguaggi come JavaScript che non hanno sovraccarico di round(value, precision) .
Ora, se vuoi introdurre una costante, come si chiamerebbe? Il termine più vicino che puoi ottenere è Precision . Quindi:
const precision = 1000;
const result = Math.round(input * precision) / precision;
Migliora la leggibilità? Può essere. Qui, il valore di una costante è piuttosto limitato e potresti chiederti se hai davvero bisogno di eseguire il refactoring. La cosa bella qui è che ora, la precisione è dichiarata solo una volta, quindi se cambia, non rischi di commettere un errore come:
const result = Math.round(input * 100) / 1000;
modifica del valore in una posizione e dimentica di farlo nell'altra.
Esempio 3
Da questi esempi, potresti avere l'impressione che i numeri dovrebbero essere sostituiti da costanti in ogni caso . Questo non è vero. In alcune situazioni, avere una costante non porta al miglioramento del codice.
Prendi la seguente parte di codice:
class Point
{
...
public void Reset()
{
x, y = (0, 0);
}
}
Se provi a sostituire gli zeri con una variabile, la difficoltà sarebbe trovare un nome significativo. Come lo chiameresti? %codice%? %codice%? %codice%? L'introduzione di una costante qui non migliorerebbe il codice in alcun modo. Lo renderebbe leggermente più lungo, e solo quello.
Tali casi sono tuttavia rari. Così ogni volta che trovi un numero nel codice, fai lo sforzo cercando di scoprire come il codice può essere rifatto. Chiediti se c'è un significato aziendale per il numero. Se sì, una costante è obbligatoria. Se no, come chiameresti il numero? Se trovi un nome significativo, è grandioso. In caso contrario, è probabile che tu abbia trovato un caso in cui la costante non è necessaria.