¿Debo agregar código redundante ahora solo en caso de que sea necesario en el futuro?

114

Bien o mal, actualmente creo que siempre debería tratar de hacer que mi código sea lo más robusto posible, incluso si esto significa agregar código / verificaciones redundantes que que no serán de ninguna utilidad en este momento, pero podría ser x cantidad de años más adelante.

Por ejemplo, actualmente estoy trabajando en una aplicación móvil que tiene este código:

public static CalendarRow AssignAppointmentToRow(Appointment app, List<CalendarRow> rows)
{
    //1. Is rows equal to null? - This will be the case if this is the first appointment.
    if (rows == null) {
        rows = new List<CalendarRow> ();
    }

    //2. Is rows empty? - This will be the case if this is the first appointment / some other unknown reason.
    if(rows.Count == 0)
    {
        rows.Add (new CalendarRow (0));
        rows [0].Appointments.Add (app);
    }

    //blah...
}

Mirando específicamente la sección dos, sé que si la sección uno es verdadera, entonces la sección dos también será verdadera. No puedo pensar en ninguna razón de por qué la sección uno sería falsa y la sección dos evalúa verdadero, lo que hace que la segunda ifdeclaración sea redundante.

Sin embargo, puede haber un caso en el futuro en el que esta segunda ifdeclaración sea realmente necesaria, y por una razón conocida.

Algunas personas pueden ver esto inicialmente y pensar que estoy programando con el futuro en mente, lo que obviamente es algo bueno. Pero sé de algunos casos en los que este tipo de código me ha "ocultado" errores. Lo que significa que me ha llevado aún más tiempo descubrir por qué la función xyzestá funcionando abccuando realmente debería estar funcionando def.

Por otro lado, también ha habido numerosos casos en los que este tipo de código ha hecho que sea mucho, mucho más fácil mejorar el código con nuevos comportamientos, porque no tengo que regresar y asegurarme de que todas las verificaciones relevantes estén en su lugar.

¿Hay alguna regla general para este tipo de código? (¿También me interesaría saber si esto se consideraría una buena o mala práctica?)

NB: Esto podría considerarse similar a esta pregunta, sin embargo , a diferencia de esa pregunta, me gustaría una respuesta asumiendo que no hay plazos.

TLDR: ¿Debería ir tan lejos como para agregar código redundante para hacerlo potencialmente más robusto en el futuro?

KidCode
fuente
95
En el desarrollo de software, cosas que nunca deberían suceder suceden todo el tiempo.
Roman Reiner
34
Si sabe if(rows.Count == 0)que nunca sucederá, entonces puede plantear una excepción cuando ocurra, y verificar por qué su suposición se equivocó.
knut
99
Irrelevante a la pregunta, pero sospecho que su código tiene un error. Cuando las filas son nulas, se crea una nueva lista y (supongo) desechada. Pero cuando las filas no son nulas, se cambia una lista existente. Un mejor diseño podría ser insistir en que el cliente pase una lista que puede estar vacía o no.
Theodore Norvell
99
¿Por qué rowsalguna vez sería nulo? Nunca hay una buena razón, al menos en .NET, para que una colección sea nula. Vacío , claro, pero no nulo . Lanzaría una excepción si rowses nulo, porque significa que hay un lapso de lógica por parte de la persona que llama.
Kyralessa

Respuestas:

176

Como ejercicio, primero verifiquemos su lógica. Aunque, como veremos, tiene problemas más grandes que cualquier problema lógico.

Llame a la primera condición A y la segunda condición B.

Primero dices:

Mirando específicamente la sección dos, sé que si la sección uno es verdadera, entonces la sección dos también será verdadera.

Es decir: A implica B, o, en términos más básicos. (NOT A) OR B

Y entonces:

No puedo pensar en ninguna razón por la cual la sección uno sería falsa y la sección dos evalúa verdadero, lo que hace que la segunda declaración if sea redundante.

Es decir: NOT((NOT A) AND B). Aplicar la ley de Demorgan para obtener (NOT B) OR Acuál es B implica A.

Por lo tanto, si ambas afirmaciones son verdaderas, A implica B y B implica A, lo que significa que deben ser iguales.

Por lo tanto, sí, las verificaciones son redundantes. Parece que tiene cuatro rutas de código a través del programa, pero de hecho solo tiene dos.

Entonces, la pregunta es: ¿cómo escribir el código? La verdadera pregunta es: ¿cuál es el contrato declarado del método ? La cuestión de si las condiciones son redundantes es una pista falsa. La verdadera pregunta es "¿he diseñado un contrato razonable y mi método implementa claramente ese contrato?"

Veamos la declaración:

public static CalendarRow AssignAppointmentToRow(
    Appointment app,    
    List<CalendarRow> rows)

Es público, por lo que tiene que ser robusto con los datos incorrectos de las personas que llaman arbitrariamente.

Devuelve un valor, por lo que debería ser útil por su valor de retorno, no por su efecto secundario.

Y, sin embargo, el nombre del método es un verbo, lo que sugiere que es útil por su efecto secundario.

El contrato del parámetro de lista es:

  • Una lista nula está bien
  • Una lista con uno o más elementos está bien
  • Una lista sin elementos es incorrecta y no debería ser posible.

Este contrato es una locura . ¡Imagínese escribir la documentación para esto! ¡Imagina escribir casos de prueba!

Mi consejo: empezar de nuevo. Esta API tiene una interfaz de máquina de dulces escrita por todas partes. (La expresión es de una vieja historia sobre las máquinas de dulces en Microsoft, donde tanto los precios como las selecciones son números de dos dígitos, y es muy fácil escribir "85", que es el precio del artículo 75, y obtienes el artículo equivocado. Dato curioso: sí, ¡lo he hecho por error cuando intentaba sacar chicle de una máquina expendedora de Microsoft!)

Aquí se explica cómo diseñar un contrato razonable:

Haga que su método sea útil para su efecto secundario o su valor de retorno, no para ambos.

No acepte tipos mutables como entradas, como listas; Si necesita una secuencia de información, tome un IEnumerable. Solo lee la secuencia; no escriba en una colección transferida a menos que sea muy claro que este es el contrato del método. Al tomar IEnumerable, le envía un mensaje a la persona que llama que no va a mutar su colección.

Nunca acepte nulos; Una secuencia nula es una abominación. Solicite a la persona que llama que pase una secuencia vacía si eso tiene sentido, nunca es nulo.

Bloquee de inmediato si la persona que llama viola su contrato, para enseñarle que se refiere a negocios y para que detecten sus errores en las pruebas, no en la producción.

Diseñe el contrato primero para que sea lo más sensato posible, y luego implemente claramente el contrato. Esa es la forma de preparar un diseño a prueba de futuro.

Ahora, solo he hablado sobre su caso específico, y usted hizo una pregunta general. Así que aquí hay algunos consejos generales adicionales:

  • Si hay un hecho que usted como desarrollador puede deducir pero el compilador no puede, entonces use una afirmación para documentar ese hecho. Si otro desarrollador, como tu futuro o uno de tus compañeros de trabajo, viola esa suposición, entonces la afirmación te lo dirá.

  • Obtenga una herramienta de cobertura de prueba. Asegúrese de que sus pruebas cubran cada línea de código. Si hay un código descubierto, entonces falta una prueba o tiene un código muerto. El código muerto es sorprendentemente peligroso porque generalmente no está destinado a estar muerto. La falla de seguridad increíblemente horrible de Apple "goto fail" de hace un par de años viene inmediatamente a mi mente.

  • Obtenga una herramienta de análisis estático. Diablos, consigue varios; Cada herramienta tiene su propia especialidad particular y ninguna es un superconjunto de las otras. Preste atención cuando le dice que hay un código inalcanzable o redundante. De nuevo, esos son probablemente errores.

Si suena como si estuviera diciendo: primero, diseñe bien el código, y segundo, pruebe el diablo para asegurarse de que sea correcto hoy, bueno, eso es lo que estoy diciendo. Hacer esas cosas hará que lidiar con el futuro sea mucho más fácil; La parte más difícil del futuro es lidiar con todo el código de la máquina de dulces con errores que la gente escribió en el pasado. Hazlo bien hoy y los costos serán más bajos en el futuro.

Eric Lippert
fuente
24
Nunca antes había pensado en métodos como este, pensando en ello ahora, siempre trato de cubrir todas las eventualidades, cuando en realidad si la persona / método que llama a mi método no me pasa lo que necesito, entonces no debería No intente solucionar sus errores (cuando no sé realmente lo que pretendían), simplemente debería colapsar. Gracias por esto, ¡se ha aprendido una valiosa lección!
KidCode
44
El hecho de que un método devuelva un valor no implica que tampoco debería ser útil por su efecto secundario. En el código concurrente, la capacidad de las funciones para devolver ambas es a menudo vital (¡imagínese CompareExchangesin esa capacidad!), E incluso en escenarios no concurrentes algo como "agregue un registro si no existe y devuelva cualquiera de los pasados record o el que existió "puede ser más conveniente que cualquier enfoque que no emplee tanto un efecto secundario como un valor de retorno.
supercat
55
@KidCode Sí, Eric es muy bueno para explicar las cosas con claridad, incluso algunos temas realmente complicados. :)
Mason Wheeler
3
@supercat Claro, pero también es más difícil razonar. Primero, es probable que desee ver algo que no modifique un estado global, evitando así problemas de concurrencia y corrupción estatal. Cuando esto no sea razonable, aún querrá aislar los dos, lo que le brinda lugares claros donde la concurrencia es un problema (y, por lo tanto, se considera más peligroso) y lugares donde se maneja. Esta fue una de las ideas centrales en el documento original de OOP, y está en el centro de la utilidad de los actores. No hay una regla religiosa, solo prefiera separarlos cuando tenga sentido. Por lo general lo hace.
Luaan
55
¡Esta es una publicación muy útil para casi todos!
Adrian Buzea
89

Lo que está haciendo en el código que muestra arriba no es tanto una prueba futura como una codificación defensiva .

Ambas ifdeclaraciones prueban cosas diferentes. Ambas son pruebas apropiadas según sus necesidades.

La sección 1 prueba y corrige un nullobjeto. Nota al margen : la creación de la lista no crea ningún elemento secundario (por ejemplo, CalendarRow).

La Sección 2 prueba y corrige los errores de usuario y / o implementación. El hecho de que tenga un List<CalendarRow>no significa que tenga algún elemento en la lista. Los usuarios y los implementadores harán cosas que no puedes imaginar solo porque se les permite hacerlo, ya sea que tenga sentido para ti o no.

Adam Zuckerman
fuente
1
En realidad estoy tomando 'trucos estúpidos de usuario' para significar entrada. SÍ, nunca debe confiar en la entrada. Incluso desde afuera de tu clase. ¡Validar! Si crees que esto es solo una preocupación futura, me encantaría hackearlo hoy.
candied_orange
1
@CandiedOrange, esa era la intención, pero la redacción no transmitía el intento de humor. He cambiado la redacción.
Adam Zuckerman
3
Solo una pregunta rápida aquí, si se trata de un error de implementación / datos incorrectos, ¿no debería simplemente fallar, en lugar de tratar de corregir sus errores?
KidCode
44
@KidCode Cada vez que intente recuperarse, "corrija sus errores", debe hacer dos cosas, volver a un estado bueno conocido y no perder información valiosa en silencio. Seguir esa regla en este caso plantea la pregunta: ¿es valiosa una lista de filas cero?
candied_orange
77
No estoy de acuerdo con esta respuesta. Si una función recibe una entrada no válida, significa que, por definición, hay un error en el programa. El enfoque correcto es lanzar una excepción en la entrada no válida, para que descubra el problema y solucione el error. El enfoque que describe va a ocultar los errores, lo que los hace más insidiosos y más difíciles de rastrear. La codificación defensiva significa que no confía automáticamente en la entrada, sino que la valida, pero no significa que deba hacer suposiciones aleatorias para "corregir" la entrada no válida o inesperada.
JacquesB
35

Supongo que esta pregunta se debe básicamente al gusto. Sí, es una buena idea escribir código robusto, sin embargo, el código en su ejemplo es una ligera violación del principio KISS (ya que muchos de esos códigos "a prueba de futuro" serán).

Es probable que personalmente no me moleste en hacer el código a prueba de balas para el futuro. No sé el futuro, y como tal, cualquier código de "futuro a prueba de balas" está condenado a fallar miserablemente cuando el futuro llegue de todos modos.

En cambio, preferiría un enfoque diferente: haga las suposiciones que hace explícitas utilizando la assert()macro o instalaciones similares. De esa manera, cuando el futuro llegue por la puerta, le dirá exactamente dónde sus suposiciones ya no se mantienen.

cmaster
fuente
44
Me gusta tu punto de no saber lo que depara el futuro. En este momento, todo lo que realmente estoy haciendo es adivinar qué podría ser un problema, y ​​luego adivinar nuevamente la solución.
KidCode
3
@KidCode: buena observación. Su propio pensamiento aquí es en realidad mucho más inteligente que muchas de las respuestas aquí, incluida la que ha aceptado.
JacquesB
1
Me gusta esta respuesta Mantenga el código mínimo para que sea fácil para un futuro lector entender por qué se requieren verificaciones. Si un futuro lector ve cheques por cosas que parecen innecesarias, puede perder el tiempo tratando de entender por qué está ahí el cheque. Un futuro humano puede estar modificando esta clase, en lugar de otros que la usan. Además, no escriba código que no pueda depurar , que será el caso si intenta manejar casos que actualmente no pueden suceder. (A menos que escriba pruebas unitarias que ejerciten rutas de código que el programa principal no hace)
Peter Cordes
23

Otro principio en el que puede pensar es la idea de fallar rápidamente . La idea es que cuando algo sale mal en su programa, desea detenerlo por completo de inmediato, al menos mientras lo está desarrollando antes de lanzarlo. Bajo este principio, desea escribir muchos cheques para asegurarse de que sus supuestos se mantengan, pero considere seriamente hacer que su programa se detenga en seco cada vez que se infrinjan los supuestos.

Para decirlo con audacia, si incluso hay un pequeño error en su programa, ¡querrá que se bloquee por completo mientras mira!

Esto puede sonar contradictorio, pero le permite descubrir errores lo más rápido posible durante el desarrollo de rutina. Si está escribiendo un código y cree que está terminado, pero se bloquea cuando lo prueba, no hay duda de que aún no ha terminado. Además, la mayoría de los lenguajes de programación le brindan excelentes herramientas de depuración que son más fáciles de usar cuando su programa se bloquea por completo en lugar de intentar hacerlo lo mejor posible después de un error. El ejemplo más grande y más común es que si bloquea su programa lanzando una excepción no controlada, el mensaje de excepción le indicará una cantidad increíble de información sobre el error, incluida la línea de código que falló y la ruta a través de su código que tomó el programa. su camino a esa línea de código (el seguimiento de la pila).

Para más ideas, lea este breve ensayo: No clave su programa en la posición vertical


Esto es relevante para usted porque es posible que a veces, los cheques que está escribiendo estén allí porque desea que su programa intente seguir funcionando incluso después de que algo salió mal. Por ejemplo, considere esta breve implementación de la secuencia de Fibonacci:

// Calculates the nth Fibonacci number
int fibonacci(int n) {
    int a = 0;
    int b = 1;

    for(int i = 0; i < n; i++) {
        int temp = b;
        b = a + b;
        a = temp;
    }

    return b;
}

Esto funciona, pero ¿qué pasa si alguien pasa un número negativo a su función? ¡Entonces no funcionará! Por lo tanto, querrá agregar una verificación para asegurarse de que la función se llame con una entrada no negativa.

Puede ser tentador escribir la función de esta manera:

// Calculates the nth Fibonacci number
int fibonacci(int n) {
    int a = 0;
    int b = 1;

    // Make sure the input is nonnegative
    if(n < 0) {
        n = 1; // Replace the negative input with an input that will work
    }

    for(int i = 0; i < n; i++) {
        int temp = b;
        b = a + b;
        a = temp;
    }

    return b;
}

Sin embargo, si hace esto, luego llama accidentalmente a su función Fibonacci con una entrada negativa, ¡nunca se dará cuenta! Peor aún, su programa probablemente continuará ejecutándose, pero comenzará a producir resultados sin sentido sin darle ninguna pista sobre dónde salió algo mal. Estos son los tipos de errores más difíciles de corregir.

En cambio, es mejor escribir un cheque como este:

// Calculates the nth Fibonacci number
int fibonacci(int n) {
    int a = 0;
    int b = 1;

    // Make sure the input is nonnegative
    if(n < 0) {
        throw new ArgumentException("Can't have negative inputs to Fibonacci");
    }

    for(int i = 0; i < n; i++) {
        int temp = b;
        b = a + b;
        a = temp;
    }

    return b;
}

Ahora, si alguna vez accidentalmente llama a la función Fibonacci con una entrada negativa, su programa se detendrá inmediatamente y le informará que algo está mal. Además, al darle un seguimiento de la pila, el programa le permitirá saber qué parte de su programa intentó ejecutar la función de Fibonacci de manera incorrecta, brindándole un excelente punto de partida para depurar lo que está mal.

Kevin
fuente
1
¿C # no tiene un tipo particular de excepción para indicar argumentos inválidos o argumentos fuera de rango?
JDługosz
@ JDługosz ¡Sí! C # tiene una excepción ArgumentException y Java tiene una excepción IllegalArgumentException .
Kevin
La pregunta estaba usando c #. Aquí está C ++ (enlace al resumen) para completar.
JDługosz
3
"Fallar rápido al estado seguro más cercano" es más razonable. Si crea su aplicación para que se bloquee cuando sucede algo inesperado, corre el riesgo de perder los datos del usuario. Los lugares donde los datos del usuario están en peligro son excelentes puntos para el manejo "penúltimo" de excepciones (siempre hay casos en los que no puede hacer nada más que levantar las manos: el bloqueo definitivo). Solo hacer que se bloquee en la depuración es simplemente abrir otra lata de gusanos: de todos modos, debe probar lo que está implementando para el usuario, y ahora está pasando la mitad de su tiempo de prueba en una versión que el usuario nunca verá.
Luaan
2
@Luaan: sin embargo, esa no es responsabilidad de la función de ejemplo.
llame el
11

¿Deberías agregar código redundante? No.

Pero, lo que describe no es código redundante .

Lo que usted describe es la programación defensiva contra el código de llamada que viola las condiciones previas de su función. Si hace esto o simplemente deja que los usuarios lean la documentación y eviten esas violaciones, es completamente subjetivo.

Personalmente, creo mucho en esta metodología, pero como con todo, hay que tener cuidado. Tomemos, por ejemplo, los de C ++ std::vector::operator[]. Dejando a un lado la implementación del modo de depuración de VS por un momento, esta función no realiza la comprobación de límites. Si solicita un elemento que no existe, el resultado no está definido. Depende del usuario proporcionar un índice vectorial válido. Esto es bastante deliberado: puede "optar" por la verificación de límites agregándolo en el sitio de llamadas, pero si la operator[]implementación lo realizara, entonces no podría "optar por no participar". Como una función de nivel bastante bajo, esto tiene sentido.

Pero si estuviera escribiendo una AddEmployee(string name)función para alguna interfaz de nivel superior, esperaría que esta función al menos arroje una excepción si proporcionó un vacío name, y esta condición previa se documenta inmediatamente después de la declaración de la función. Es posible que hoy no proporcione información de usuario no desinfectada a esta función, pero hacerla "segura" de esta manera significa que cualquier infracción de condición previa que surja en el futuro puede diagnosticarse fácilmente, en lugar de desencadenar potencialmente una cadena de fichas de dominó de difícil acceso. detectar errores Esto no es redundancia: es diligencia.

Si tuviera que llegar a una regla general (aunque, como regla general, trato de evitarlas), diría que es una función que satisface cualquiera de los siguientes:

  • vive en un lenguaje de nivel súper alto (por ejemplo, JavaScript en lugar de C)
  • se sienta en un límite de interfaz
  • no es crítico para el rendimiento
  • acepta directamente la entrada del usuario

... puede beneficiarse de la programación defensiva. En otros casos, aún puede escribir assertiones que se disparan durante las pruebas pero que están desactivados en las versiones de lanzamiento, para aumentar aún más su capacidad de encontrar errores.

Este tema se explora más a fondo en Wikipedia ( https://en.wikipedia.org/wiki/Defensive_programming ).

Carreras de ligereza en órbita
fuente
9

Dos de los diez mandamientos de programación son relevantes aquí:

  • No debes asumir que la entrada es correcta

  • No deberá hacer código para uso futuro

Aquí, verificar nulo no es "hacer código para uso futuro". Hacer código para uso futuro es algo como agregar interfaces porque cree que podrían ser útiles "algún día". En otras palabras, el mandamiento es no agregar capas de abstracción a menos que sean necesarias en este momento.

Comprobar nulo no tiene nada que ver con el uso futuro. Tiene que ver con el Mandamiento # 1: no asuma que la entrada será correcta. Nunca suponga que su función recibirá algún subconjunto de entrada. Una función debe responder de manera lógica, sin importar cuán falsas y desordenadas sean las entradas.

Tyler Durden
fuente
55
¿Dónde están estos mandamientos de programación? ¿Tienes un enlace? Tengo curiosidad porque he trabajado en varios programas que se suscriben al segundo de esos mandamientos, y varios que no lo hicieron. Invariablemente, aquellos que se suscribieron al mandamiento se Thou shall not make code for future useencontraron con problemas de mantenimiento antes , a pesar de la lógica intuitiva del mandamiento. He encontrado, en la codificación de la vida real, que el mandamiento solo es efectivo en el código en el que controlas la lista de características y los plazos, y me aseguro de que no necesites un código de futuro para alcanzarlos ... es decir, nunca.
Cort Ammon
1
Trivialmente demostrable: si se puede estimar la probabilidad de uso futuro y el valor proyectado del código de "uso futuro", y el producto de esos dos es mayor que el costo de agregar el código de "uso futuro", es estadísticamente óptimo para agrega el código. Creo que el mandamiento aparece en situaciones en las que los desarrolladores (o gerentes) se ven obligados a admitir que sus habilidades de estimación no son tan confiables como les gustaría, por lo que, como medida defensiva, simplemente eligen no estimar la tarea futura.
Cort Ammon
2
@CortAmmon No hay lugar para los mandamientos religiosos en la programación: las "mejores prácticas" solo tienen sentido en el contexto, y aprender una "mejor práctica" sin el razonamiento te hace incapaz de adaptarte. He encontrado que YAGNI es muy útil, pero eso no significa que no piense en lugares donde agregar puntos de extensión más tarde será costoso, solo significa que no tengo que pensar en los casos simples con anticipación. Por supuesto, esto también cambia con el tiempo, ya que cada vez más suposiciones se atornillan a su código, aumentando efectivamente la interfaz de su código, es un acto de equilibrio.
Luaan
2
@CortAmmon Su caso "trivialmente demostrable" ignora dos costos muy importantes: el costo de la estimación en sí y los costos de mantenimiento del punto de extensión (posiblemente innecesario). Ahí es donde las personas obtienen estimaciones extremadamente poco confiables, subestimando las estimaciones. Para una característica muy simple, puede ser suficiente pensar por unos segundos, pero es muy probable que encuentre una lata completa de gusanos que se desprende de la característica inicialmente "simple". La comunicación es la clave: a medida que las cosas se hacen más grandes, debe hablar con su líder / cliente.
Luaan
1
@Luaan Estaba tratando de defender tu punto, no hay lugar para los mandamientos religiosos en la programación. Mientras exista un caso de negocios en el que los costos de estimación y mantenimiento del punto de extensión estén suficientemente limitados, hay un caso en el que dicho "mandamiento" es cuestionable. Desde mi experiencia con el código, la cuestión de si dejar esos puntos de extensión o no nunca ha encajado bien en un solo comando de línea de una manera u otra.
Cort Ammon
7

La definición de 'código redundante' y 'YAGNI' a menudo depende de lo lejos que mire hacia adelante.

Si experimenta un problema, entonces tiende a escribir código futuro de tal manera que evite ese problema. Otro programador que no haya experimentado ese problema en particular podría considerar que su código es excesivo.

Mi sugerencia es hacer un seguimiento de cuánto tiempo pasas en "cosas que aún no han errado" si se carga y tus compañeros están eliminando funciones más rápido que tú, y luego disminuirlo.

Sin embargo, si eres como yo, espero que lo hayas escrito todo 'por defecto' y realmente no te haya tomado más tiempo.

Ewan
fuente
6

Es una buena idea documentar cualquier suposición sobre los parámetros. Y es una buena idea verificar que su código de cliente no viole esas suposiciones. Yo haría esto:

/** ...
*   Precondition: rows is null or nonempty
*/
public static CalendarRow AssignAppointmentToRow(Appointment app, List<CalendarRow> rows)
{
    Assert( rows==null || rows.Count > 0 )
    //1. Is rows equal to null? - This will be the case if this is the first appointment.
    if (rows == null) {
        rows = new List<CalendarRow> ();
        rows.Add (new CalendarRow (0));
        rows [0].Appointments.Add (app);
    }

    //blah...
}

[Asumiendo que esto es C #, Assert podría no ser la mejor herramienta para el trabajo, ya que no está compilado en el código publicado. Pero ese es un debate para otro día.]

¿Por qué es esto mejor que lo que escribiste? Su código tiene sentido si, en un futuro donde su cliente ha cambiado, cuando el cliente pasa en una lista vacía, lo correcto será agregar una primera fila y agregar una aplicación a sus citas. Pero, ¿cómo sabes que ese será el caso? Es mejor hacer menos suposiciones ahora sobre el futuro.

Theodore Norvell
fuente
5

Calcule el costo de agregar ese código ahora . Será relativamente barato porque todo está fresco en su mente, por lo que podrá hacerlo rápidamente. Es necesario agregar pruebas unitarias: no hay nada peor que usar algún método un año después, no funciona y te das cuenta de que se rompió desde el principio y nunca funcionó.

Calcule el costo de agregar ese código cuando sea necesario. Será más costoso, porque tienes que volver al código, recordarlo todo, y es mucho más difícil.

Estime la probabilidad de que el código adicional sea realmente necesario. Entonces haz las matemáticas.

Por otro lado, el código completo con suposiciones "X nunca sucederá" es horrible para la depuración. Si algo no funciona según lo previsto, significa un error estúpido o una suposición errónea. Su "X nunca sucederá" es una suposición, y en presencia de un error es sospechoso. Lo que obliga al próximo desarrollador a perder el tiempo. Por lo general, es mejor no confiar en tales supuestos.

gnasher729
fuente
44
En su primer párrafo, olvidó mencionar el costo de mantener ese código a lo largo del tiempo, cuando resulta que la característica que realmente se necesita es mutuamente exclusiva con la característica que se agregó innecesariamente. . .
ruakh
También debe estimar el costo de los errores que pueden aparecer en el programa porque no falla en la entrada no válida. Pero no puede estimar el costo de los errores ya que, por definición, son inesperados. Entonces todo el "hacer las matemáticas" se desmorona.
JacquesB
3

La pregunta principal aquí es "¿qué pasará si haces / no haces?"

Como otros han señalado, este tipo de programación defensiva es buena, pero ocasionalmente también es peligrosa.

Por ejemplo, si proporciona un valor predeterminado, entonces mantiene el programa funcionando. Pero el programa ahora puede no estar haciendo lo que quieres que haga. Por ejemplo, si escribe esa matriz en blanco en un archivo, es posible que haya convertido su error de "bloqueos porque proporcioné nulo por accidente" a "borra las filas del calendario porque proporcioné nulo por accidente". (por ejemplo, si comienza a eliminar cosas que no aparecen en la lista en la parte que dice "// blah")

La clave para mí es nunca corromper datos . Déjame repetir eso; NUNCA. CORRUPTO. DATOS. Si las excepciones de su programa, obtiene un informe de error que puede parchear; Si escribe datos incorrectos en un archivo que luego hará, debe sembrar el suelo con sal.

Todas sus decisiones "innecesarias" deben tomarse con esa premisa en mente.

deworde
fuente
2

Aquí se trata básicamente de una interfaz. Al agregar el comportamiento "cuando la entrada es null, inicializar la entrada", ha extendido efectivamente la interfaz del método; ahora, en lugar de operar siempre en una lista válida, ha logrado "arreglar" la entrada. Si esta es la parte oficial o no oficial de la interfaz, puede apostar que alguien (probablemente incluyéndolo a usted) va a usar este comportamiento.

Las interfaces deben mantenerse simples y deben ser relativamente estables, especialmente en algo así como un public staticmétodo. Tienes un poco de margen de maniobra en los métodos privados, especialmente los métodos de instancias privadas. Al extender implícitamente la interfaz, ha hecho que su código sea más complejo en la práctica. Ahora, imagine que en realidad no desea usar esa ruta de código, así que evítela. Ahora tienes un poco de código no probado que finge que es parte del comportamiento del método. Y puedo decirle ahora que probablemente tiene un error: cuando pasa una lista, esa lista está mutada por el método. Sin embargo, si no lo hace, crea un locallista, y tírelo más tarde. Este es el tipo de comportamiento inconsistente que te hará llorar en medio año, mientras tratas de localizar un error oscuro.

En general, la programación defensiva es algo bastante útil. Pero las rutas de código para las comprobaciones defensivas deben probarse, como cualquier otro código. En un caso como este, están complicando su código sin ninguna razón, y optaría por una alternativa como esta:

if (rows == null) throw new ArgumentNullException(nameof(rows));

No desea una entrada donde rowssea ​​nulo, y desea hacer que el error sea obvio para todas las personas que llaman lo antes posible .

Hay muchos valores que necesita hacer malabares al desarrollar software. Incluso la robustez en sí misma es una cualidad muy compleja; por ejemplo, no consideraría su control defensivo más robusto que lanzar una excepción. Las excepciones son bastante útiles para brindarle un lugar seguro para volver a intentarlo desde un lugar seguro: los problemas de corrupción de datos suelen ser mucho más difíciles de rastrear que reconocer un problema desde el principio y manejarlo de manera segura. Al final, solo tienden a darle una ilusión de robustez, y luego, un mes después, nota que una décima parte de sus citas se han ido, porque nunca notó que se actualizó una lista diferente. Ay.

Asegúrese de distinguir entre los dos. La programación defensiva es una técnica útil para detectar errores en un lugar donde son los más relevantes, lo que ayuda significativamente a sus esfuerzos de depuración y, con un buen manejo de excepciones, evita la "corrupción furtiva". Falla temprano, falla rápido. Por otro lado, lo que estás haciendo es más como "ocultar errores": estás haciendo malabares con las entradas y haciendo suposiciones sobre lo que quiso decir la persona que llama. Esto es muy importante para el código orientado al usuario (por ejemplo, la corrección ortográfica), pero debe tener cuidado al ver esto con el código orientado al desarrollador.

El problema principal es que cualquier abstracción que hagas, se filtrará ("¡Quería escribir de, no antes! ¡Corrector ortográfico estúpido!"), Y el código para manejar todos los casos especiales y arreglos sigue siendo código necesita mantener y comprender, y el código que necesita probar. Compare el esfuerzo de asegurarse de que se apruebe una lista no nula con la corrección del error que tiene un año después en la producción; no es una buena compensación. En un mundo ideal, desearía que cada método funcionara exclusivamente con su propia entrada, devolviendo un resultado y sin modificar el estado global. Por supuesto, en el mundo real, encontrarás muchos casos en los que eso no eses la solución más simple y clara (por ejemplo, al guardar un archivo), pero creo que mantener los métodos "puros" cuando no hay razón para que lean o manipulen el estado global hace que el código sea mucho más fácil de razonar. También tiende a darte puntos más naturales para dividir tus métodos :)

Esto no significa que todo lo inesperado deba hacer que su aplicación se bloquee, por el contrario. Si usa bien las excepciones, naturalmente forman puntos seguros de manejo de errores, donde puede restaurar el estado estable de la aplicación y permitir que el usuario continúe con lo que está haciendo (idealmente mientras evita cualquier pérdida de datos para el usuario). En esos puntos de manejo, verá oportunidades para solucionar los problemas ("No se encontró el número de orden 2212. ¿Se refería a 2212b?") O dar control al usuario ("Error al conectarse a la base de datos. ¿Intentar de nuevo?"). Incluso cuando no hay tal opción disponible, al menos le dará la oportunidad de que nada se corrompa: comencé a apreciar el código que usa usingy try... finallymucho más que try...catch, le brinda muchas oportunidades para mantener invariantes incluso en condiciones excepcionales.

Los usuarios no deben perder sus datos y su trabajo. Esto todavía tiene que equilibrarse con los costos de desarrollo, etc., pero es una guía general bastante buena (si los usuarios deciden si comprar su software o no, el software interno generalmente no tiene ese lujo). Incluso el bloqueo total de la aplicación se convierte en un problema mucho menor si el usuario puede simplemente reiniciar y volver a lo que estaba haciendo. Esto es realmente robusto: Word guarda su trabajo todo el tiempo sin corromper su documento en el disco y darle una opciónpara restaurar esos cambios después de reiniciar Word después de un bloqueo. ¿Es mejor que un no bug en primer lugar? Probablemente no, aunque no olvide que el trabajo dedicado a la captura de un error raro podría ser mejor gastado en todas partes. Pero es mucho mejor que las alternativas: por ejemplo, un documento dañado en el disco, todo el trabajo desde la última vez que se guardó, el documento se reemplazó automáticamente con cambios antes del bloqueo, que simplemente fueron Ctrl + A y Eliminar.

Luaan
fuente
1

Voy a responder esto basado en su suposición de que un código robusto lo beneficiará "años" a partir de ahora. Si su objetivo son los beneficios a largo plazo, priorizaría el diseño y la mantenibilidad sobre la solidez.

La compensación entre diseño y robustez es tiempo y enfoque. La mayoría de los desarrolladores preferirían tener un conjunto de código bien diseñado, incluso si eso significa pasar por algunos puntos problemáticos y hacer alguna condición adicional o manejo de errores. Después de algunos años de uso, los lugares que realmente lo necesita probablemente hayan sido identificados por los usuarios.

Asumiendo que el diseño tiene aproximadamente la misma calidad, menos código es más fácil de mantener. Esto no significa que estemos mejor si dejas pasar los problemas conocidos durante algunos años, pero agregar cosas que sabes que no necesitas hace que sea difícil. Todos hemos examinado el código heredado y hemos encontrado partes innecesarias. Debe tener un código de cambio de confianza de alto nivel que haya funcionado durante años.

Por lo tanto, si cree que su aplicación está diseñada de la mejor manera posible, es fácil de mantener y no tiene errores, busque algo mejor que agregar un código que no necesita. Es lo menos que puede hacer por respeto a todos los demás desarrolladores que trabajan largas horas en funciones inútiles.

JeffO
fuente
1

No, no deberías. Y en realidad estás respondiendo tu propia pregunta cuando afirmas que esta forma de codificación puede ocultar errores . Esto no hará que el código sea más robusto, sino que lo hará más propenso a errores y dificultará la depuración.

Usted declara sus expectativas actuales sobre el rowsargumento: es nulo o tiene al menos un elemento. Entonces la pregunta es: ¿es una buena idea escribir el código para manejar adicionalmente el tercer caso inesperado donde rowstiene cero elementos?

La respuesta es no. Siempre debe lanzar una excepción en el caso de una entrada inesperada. Considere esto: si algunas otras partes del código rompen la expectativa (es decir, el contrato) de su método , significa que hay un error . Si hay un error, desea saberlo lo antes posible para poder solucionarlo, y una excepción lo ayudará a hacerlo.

Lo que el código hace actualmente es adivinar cómo recuperarse de un error que puede existir o no en el código. Pero incluso si hay un error, no puede saber cómo recuperarse completamente de él. Los errores, por definición, tienen consecuencias desconocidas. Quizás algún código de inicialización no se ejecutó como se esperaba, ya que puede tener muchas otras consecuencias además de la fila que falta.

Entonces su código debería verse así:

public static CalendarRow AssignAppointmentToRow(Appointment app, List<CalendarRow> rows)
{
    if (rows != null && rows.Count == 0) throw new ArgumentException("Rows is empty."); 

    //1. Is rows equal to null? - This will be the case if this is the first appointment.
    if (rows == null) {
        rows = new List<CalendarRow> ();
        rows.Add (new CalendarRow (0));
        rows [0].Appointments.Add (app);
    }

    //blah...
}

Nota: Hay algunos casos específicos en los que tiene sentido "adivinar" cómo manejar la entrada no válida en lugar de simplemente lanzar una excepción. Por ejemplo, si maneja la entrada externa no tiene control sobre. Los navegadores web son un ejemplo infame porque intentan manejar con gracia cualquier tipo de entrada con formato incorrecto o no válido. Pero esto solo tiene sentido con la entrada externa, no con llamadas de otras partes del programa.


Editar: Algunas otras respuestas indican que estás haciendo programación defensiva . Estoy en desacuerdo. La programación defensiva significa que no confía automáticamente en que la entrada sea válida. Entonces, la validación de los parámetros (como arriba) es una técnica de programación defensiva, pero no significa que deba alterar la entrada inesperada o inválida al adivinar. El enfoque defensivo robusto es validar la entrada y luego lanzar una excepción en caso de entrada inesperada o inválida.

JacquesB
fuente
1

¿Debo agregar código redundante ahora solo en caso de que sea necesario en el futuro?

No debe agregar código redundante en ningún momento.

No debe agregar código que solo se necesita en el futuro.

Debes asegurarte de que tu código se comporte bien sin importar lo que pase.

La definición de "comportarse bien" depende de sus necesidades. Una técnica que me gusta usar son las excepciones de "paranoia". Si estoy 100% seguro de que un determinado caso nunca puede suceder, sigo programando una excepción, pero lo hago de una manera que a) les dice claramente a todos que nunca espero que esto suceda, yb) se muestra claramente y se registra y se registra por lo tanto, no conduce a una corrupción progresiva más adelante.

Pseudocódigo de ejemplo:

file = File.open(">", "bla")  or raise "Paranoia: cannot open file 'bla'"

file.write("xytz") or raise "Paranoia: disk full?"

file.close()  or raise "Paranoia: huh?!?!?"

Esto comunica claramente que estoy 100% seguro de que siempre puedo abrir, escribir o cerrar el archivo, es decir, no llego al extremo de crear un manejo de errores elaborado. Pero si (no: cuándo) no puedo abrir el archivo, mi programa seguirá fallando de forma controlada.

La interfaz de usuario, por supuesto, no mostrará dichos mensajes al usuario, se registrarán internamente junto con el seguimiento de la pila. Nuevamente, estas son excepciones internas de "Paranoia" que solo aseguran que el código "se detenga" cuando ocurre algo inesperado. Este ejemplo es un poco complicado, en la práctica, por supuesto, implementaría el manejo de errores reales para errores al abrir archivos, ya que esto sucede regularmente (nombre de archivo incorrecto, memoria USB montada de solo lectura, lo que sea).

Un término de búsqueda relacionado muy importante sería "fallar rápido", como se señaló en otras respuestas, y es muy útil para crear software robusto.

AnoE
fuente
-1

Aquí hay muchas respuestas demasiado complicadas. Probablemente hizo esta pregunta porque no se sentía bien acerca de ese código de pieza, pero no estaba seguro de por qué o cómo solucionarlo. Entonces mi respuesta es que el problema es muy probable en la estructura del código (como siempre).

Primero, el encabezado del método:

public static CalendarRow AssignAppointmentToRow(Appointment app, List<CalendarRow> rows)

¿Asignar cita a qué fila? Eso debería quedar claro inmediatamente de la lista de parámetros. Sin ningún conocimiento más lejos, yo esperaría que los parametros método para el siguiente aspecto: (Appointment app, CalendarRow row).

A continuación, las "verificaciones de entrada":

//1. Is rows equal to null? - This will be the case if this is the first appointment.
if (rows == null) {
    rows = new List<CalendarRow> ();
}

//2. Is rows empty? - This will be the case if this is the first appointment / some other unknown reason.
if(rows.Count == 0)
{
    rows.Add (new CalendarRow (0));
    rows [0].Appointments.Add (app);
}

Esto es una mierda.

  1. check) La persona que llama al método solo debe asegurarse de que no pase valores no inicializados dentro del método. Es la responsabilidad de un programador (intentar) no ser estúpido.
  2. check) Si no tengo en cuenta que pasar rowsal método probablemente sea incorrecto (ver el comentario anterior), entonces no debería ser responsabilidad del método llamado AssignAppointmentToRowmanipular filas de otra manera que no sea asignar la cita en algún lugar.

Pero todo el concepto de asignar citas en algún lugar es extraño (a menos que sea una parte GUI del código). Su código parece contener (o al menos intenta) una estructura de datos explícita que representa un calendario (es decir, List<CalendarRows><- que debería definirse como en Calendaralgún lugar si desea ir de esta manera, entonces estaría pasando Calendar calendara su método). Si sigue este camino, esperaría calendarque se llenen previamente con ranuras donde coloque (asigne) las citas después (por ejemplo calendar[month][day] = appointment, sería el código apropiado). Pero luego también puede optar por deshacerse de la estructura del calendario de la lógica principal por completo y simplemente tener List<Appointment>dónde los Appointmentatributos contienen atributosdate. Y luego, si necesita representar un calendario en algún lugar de la GUI, puede crear esta estructura de 'calendario explícito' justo antes de la representación.

No conozco los detalles de su aplicación, por lo que parte de esto quizás no sea aplicable para usted, pero ambas comprobaciones (principalmente la segunda) me dicen que algo en algún lugar está mal con la separación de preocupaciones en su código.

clima
fuente
-2

Para simplificar, supongamos que eventualmente necesitará este código en N días (no más tarde o antes) o no lo necesitará en absoluto.

Pseudocódigo:

let C_now   = cost of implementing the piece of code now
let C_later = ... later
let C_maint = cost of maintaining the piece of code one day
              (note that it can be negative)
let P_need  = probability that you will need the code after N days

if C_now + C_maint * N < P_need*C_later then implement the code else omit it.

Factores para C_maint:

  • ¿Mejora el código en general, haciéndolo más autodocumentado, más fácil de probar? En caso afirmativo, negativo C_maintesperado
  • ¿Hace el código más grande (por lo tanto, más difícil de leer, más largo para compilar, implementar pruebas, etc.)?
  • ¿Hay refactorizaciones / rediseños pendientes? Si es así, golpea C_maint. Este caso requiere una fórmula más compleja, con más variables de tiempo como N.

Se debe omitir algo tan grande que solo reduce el código y podría ser necesario solo en 2 años con baja probabilidad, pero una pequeña cosa que también produce afirmaciones útiles y un 50% de que será necesario en 3 meses, debe implementarse .

Vi0
fuente
También debe tener en cuenta el costo de los errores que pueden aparecer en el programa porque no rechaza las entradas no válidas. Entonces, ¿cómo calcula el costo de posibles errores difíciles de encontrar?
JacquesB