Código limpio: ¿Debo cambiar el literal 1 a una constante?

No. En ese ejemplo, 1 es perfectamente significativo.

Sin embargo, ¿qué pasa si persons.size () es cero? Parece extraño que persons.getMoney() funcione para 0 y 2 pero no para 1.

¿Por qué un fragmento de código contiene ese valor literal en particular?

• ¿Este valor tiene un significado especial en el dominio del problema ?
• ¿O es este valor solo un detalle de implementación , donde ese valor es una consecuencia directa del código que lo rodea?

Si el valor literal tiene un significado que no está claro en el contexto, entonces sí, es útil darle un nombre a ese valor a través de una constante o variable. Más adelante, cuando se olvida el contexto original, el código con nombres de variables significativos será más fácil de mantener. Recuerde, la audiencia de su código no es principalmente el compilador (el compilador funcionará felizmente con un código horrible), sino los futuros mantenedores de ese código: quién apreciará si el código es algo autoexplicativo.

• En su primer ejemplo, el significado de literales como 34 , 4 , 5 no es evidente en el contexto. En cambio, algunos de estos valores tienen un significado especial en su dominio de problema. Por lo tanto, fue bueno darles nombres.

• En su segundo ejemplo, el significado del literal 1 es muy claro en el contexto. La introducción de un nombre no es útil.

De hecho, la introducción de nombres para valores obvios también puede ser mala, ya que oculta el valor real.

• Esto puede ocultar errores si el valor nombrado se modificó o fue incorrecto, especialmente si la misma variable se reutiliza en fragmentos de código no relacionados.

• Un fragmento de código también podría funcionar bien para un valor específico, pero podría ser incorrecto en el caso general. Al introducir una abstracción innecesaria, el código ya no es obviamente correcto.

No hay límite de tamaño en los literales "obvios" porque esto depende totalmente del contexto. P.ej. el literal 1024 puede ser totalmente obvio en el contexto del cálculo del tamaño del archivo, o el literal 31 en el contexto de una función hash, o el literal padding: 0.5em en el contexto de una hoja de estilo CSS.     

Hay varios problemas con este código, que, por cierto, se pueden acortar de la siguiente manera:

public List<Money> getMoneyByPersons() {
    return persons.size() == 1 ?
        moneyService.getMoneyIfHasOnePerson() :
        moneyService.getMoney(); 
}

1. No está claro por qué una persona es un caso especial. Supongo que hay una regla comercial específica que dice que obtener dinero de una persona es radicalmente diferente de obtener dinero de varias personas. Sin embargo, tengo que buscar tanto getMoneyIfHasOnePerson como getMoney , esperando entender por qué hay casos distintos.

2. El nombre getMoneyIfHasOnePerson no se ve bien. Por el nombre, esperaría el método para verificar si hay una sola persona y, si este es el caso, obtener dinero de él; de lo contrario, no hacer nada. Desde tu código, esto no es lo que está sucediendo (o estás haciendo la condición dos veces).

3. ¿Hay alguna razón para devolver un List<Money> en lugar de una colección?

Volviendo a tu pregunta, porque no está claro por qué hay un tratamiento especial para una persona, el dígito uno debe reemplazarse por una constante, a menos que haya otra. Manera de hacer las reglas explícitas. Aquí, uno no es muy diferente de cualquier otro número mágico. Podría tener reglas comerciales que indiquen que el tratamiento especial se aplica a una, dos o tres personas, o solo a más de doce personas.

  

¿Cómo puedo distinguir el contexto para elegir la mejor solución?

Usted hace lo que hace que su código sea más explícito.

Ejemplo 1

Imagina el siguiente fragmento de código:

if (sequence.size() == 0) {
    return null;
}

return this.processSequence(sequence);

¿Es cero aquí un valor mágico? El código es bastante claro: si no hay elementos en la secuencia, no lo procesemos y devolvamos un valor especial. Pero este código también se puede reescribir así:

if (sequence.isEmpty()) {
    return null;
}

return this.processSequence(sequence);

Aquí, no más constante, y el código es aún más claro.

Ejemplo 2

Toma otra pieza de código:

const result = Math.round(input * 1000) / 1000;

No lleva mucho tiempo entender lo que hace en lenguajes como JavaScript que no tienen una sobrecarga de round(value, precision) .

Ahora, si quieres introducir una constante, ¿cómo se llamará? El término más cercano que puede obtener es Precision . Entonces:

const precision = 1000;
const result = Math.round(input * precision) / precision;

¿Mejora la legibilidad? Puede ser. Aquí, el valor de una constante es bastante limitado, y puede preguntarse si realmente necesita realizar la refactorización. Lo bueno aquí es que ahora, la precisión se declara solo una vez, por lo que si cambia, no se arriesgue a cometer un error como:

const result = Math.round(input * 100) / 1000;

cambiando el valor en una ubicación, y olvidando hacerlo en la otra.

Ejemplo 3

De esos ejemplos, puede tener la impresión de que los números deben reemplazarse por constantes en cada caso . Esto no es verdad. En algunas situaciones, tener una constante no conduce a la mejora del código.

Toma el siguiente fragmento de código:

class Point
{
    ...
    public void Reset()
    {
        x, y = (0, 0);
    }
}

Si intenta reemplazar los ceros por una variable, la dificultad sería encontrar un nombre significativo. ¿Cómo lo llamarías? %código%? %código%? %código%? Introducir una constante aquí no mejoraría el código de ninguna manera. Lo haría un poco más largo, y solo eso.

Sin embargo, estos casos son raros. Por lo tanto, cada vez que encuentre un número en el código, intente encontrar la manera de refactorizar el código. Pregúntese si hay un significado comercial para el número. Si es así, una constante es obligatoria. Si no, ¿cómo nombrarías el número? Si encuentras un nombre significativo, eso es genial. De lo contrario, es probable que encuentre un caso en el que la constante no sea necesaria.

Podrías hacer una función que tome un solo parámetro y devuelva cuatro veces dividido por cinco que ofrezca un alias limpio en lo que hace mientras se usa el primer ejemplo.

Solo ofrezco mi propia estrategia habitual, pero quizás también aprenda algo.

Lo que estoy pensando es.

// Just an example name  
function normalize_task_value(task) {  
    return (task * 4) / 5;  // or to 'return (task * 4) / NUMBER_OF_TASKS' but it really matters what logic you want to convey and there are reasons to many ways to make this logical. A comment would be the greatest improvement

}  

// Is it possible to just use tasks.length or something like that?  
// this NUMBER_OF_TASKS is the only one thats actually tricky to   
// understand right now how it plays its role.  

function normalize_all_task_values(tasks, accumulator) {  
    for (int i = 0; i < NUMBER_OF_TASKS; i++) {  
        accumulator += normalize_task_value(tasks[i]);
    }
}  

Lo siento si estoy fuera de lugar pero solo soy un desarrollador de JavaScript. No estoy seguro de qué composición justifiqué esto, supongo que no todo tiene que estar en una matriz o lista, pero .length tendría mucho sentido. Y el * 4 haría que la buena constante, ya que su origen es nebuloso.

Ese número 1, ¿podría ser un número diferente? ¿Podría ser 2, o 3, o hay razones lógicas por las que debe ser 1? Si debe ser 1, entonces usar 1 está bien. De lo contrario, puede definir una constante. No lo llames UNO constante. (He visto eso hecho).

60 segundos en un minuto: ¿necesitas una constante? Bueno, es 60 segundos, no 50 o 70. Y todos lo saben. Así que eso puede ser un número.

60 elementos impresos por página: ese número podría haber sido fácilmente 59 o 55 o 70. En realidad, si cambia el tamaño de la fuente, puede llegar a ser 55 o 70. Así que aquí se pide más una constante significativa.

También es una cuestión de lo claro que es el significado. Si escribes "minutos = segundos / 60", eso está claro. Si escribes "x = y / 60", no está claro. Debe haber algunos nombres significativos en algún lugar.

Hay una regla absoluta: no hay reglas absolutas. Con la práctica, descubrirás cuándo usar los números y cuándo usar las constantes nombradas. No lo hagas porque lo diga un libro, hasta que entiendas por qué dice eso.

He visto una buena cantidad de código como en el OP (modificado) en las recuperaciones de DB. La consulta devuelve una lista, pero las reglas comerciales dicen que solo puede haber un elemento. Y luego, por supuesto, algo cambió por "solo este caso" a una lista con más de un elemento. (Sí, lo dije una buena cantidad de veces. Es casi como que ... nm)

Entonces, en lugar de crear una constante, crearía (en un método de código limpio) un método para dar un nombre o aclarar qué pretende el condicional detectar (y resumir cómo lo detecta):

public int getMoneyByPersons(){
  if(isSingleDepartmentHead()){ 
    // TODO - return money for one person
  } else {
    // TODO - calculate and return money for people.
  }
}

public boolean isSingleDepartmentHead() {
   return persons.size() == 1;
}