Estaba leyendo en esta publicación de blog sobre el antipatrón for-if, y no estoy muy seguro de entender por qué es un antipatrón.
foreach (string filename in Directory.GetFiles("."))
{
if (filename.Equals("desktop.ini", StringComparison.OrdinalIgnoreCase))
{
return new StreamReader(filename);
}
}
Pregunta 1:
¿Es por return new StreamReader(filename);
dentro for loop
? o el hecho de que no necesitas un for
bucle en este caso?
Como señaló el autor del blog, la versión menos loca de esto es:
if (File.Exists("desktop.ini"))
{
return new StreamReader("desktop.ini");
}
ambos sufren una condición de carrera porque si el archivo se elimina antes de la creación de la StreamReader
, obtendrá un FileNotFoundException
.
Pregunta 2:
Para arreglar el segundo ejemplo, ¿lo volvería a escribir sin la instrucción if, y en su lugar lo rodearía StreamReader
con un bloque try-catch, y si arroja un, FileNotFoundException
lo maneja en el catch
bloque en consecuencia?
To fix the second example, would you re-write it without the if statement, and instead surround the StreamReader with a try-catch block, and if it throws a FileNotFoundException you handle it in the catch block accordingly?
- Sí, eso es exactamente lo que haría. Resolver la condición de carrera es más importante que alguna noción de "excepciones como flujo de control", y lo resuelve de manera elegante y limpia.null
? Por lo general, puede usar LINQ para limpiar el código que se parece al código de su ejemplo:return Directory.GetFiles(".").FirstOrDefault(fileName => fileName.Equals("desktop.ini", StringComparison.OrdinalIgnoreCase))?.Select(fileName => new StreamReader(filename));
observe el?.
operador entre las dos llamadas de LINQ. También la gente puede argumentar que la creación de objetos como esta no es el uso más apropiado de LINQ, pero creo que está bien aquí. Esta no es una respuesta a su pregunta, pero se desvía de una parte de ella.Respuestas:
Este es un antipatrón ya que toma la forma de:
y puede ser reemplazado con
Un ejemplo clásico de esto es código como:
Cuando lo siguiente funciona bien:
Si te encuentras realizando un ciclo y una
if
oswitch
, entonces detente y piensa en lo que estás haciendo. ¿Estás complicando demasiado las cosas? ¿Se puede reemplazar todo el ciclo y la prueba con una simple línea de "solo hazlo"? Sin embargo, a veces, encontrará que necesita hacer ese bucle (por ejemplo, más de un elemento puede coincidir con una condición), en cuyo caso el patrón está bien.Es por eso que es un antipatrón: toma el patrón de "bucle y prueba" y lo abusa.
Con respecto a su segunda pregunta: sí. Un patrón "probar hacer" es más robusto que un patrón "probar luego hacer" en cualquier situación en la que su código no sea el único hilo en todo el dispositivo que pueda cambiar el estado del elemento bajo prueba.
El problema con este código:
es que en el tiempo entre
File.Exists
eStreamReader
intentar abrir ese archivo, otro hilo o proceso podría eliminar el archivo. Entonces obtendrás una excepción. Por lo tanto, esa excepción debe protegerse mediante algo como:@Flater plantea un buen punto? ¿Es esto en sí mismo un antipatrón? ¿Estamos utilizando excepciones como flujo de control?
Si el código lee algo como:
entonces estaría usando excepciones como un goto glorificado y de hecho sería un antipatrón. Pero en este caso, solo estamos trabajando en el comportamiento indeseable de crear una nueva secuencia, por lo que este no es un ejemplo de ese antipatrón. Pero es un ejemplo de trabajar alrededor de ese antipatrón.
Por supuesto, lo que realmente queremos es una forma más elegante de crear la transmisión. Algo como:
Y recomendaría envolver ese
try catch
código en un método de utilidad como este si te encuentras usando mucho este código.fuente
Try
, por ejemploreturn Try(() => new StreamReader("desktop.ini")).OrElse(null);
, pero C # no admite esa construcción (sin usar una biblioteca de terceros), por lo que tenemos que trabajar con la versión torpe.File.Open
arrojará uno si no puede encontrar el archivo. Como ya tenemos una excepción excepcional, es necesario atraparla como parte del flujo de control (a menos que uno simplemente quiera dejar que la aplicación se bloquee).Maybe<Stream>
tipo que regresanothing
si el archivo no existe y una secuencia si lo hace.Sí, porque no necesita hacer su propia búsqueda de elementos almacenados en un subsistema consultable.
En resumen, el sistema de archivos, como una base de datos, es capaz de responder consultas. Cuando interactuamos con un subsistema consultable, tenemos una elección fundamental en cuanto a si queremos que dicho subsistema nos enumere sus contenidos para que podamos hacer la correspondencia fuera del subsistema, o para usar las capacidades de consulta nativas del subsistema.
Supongamos por un momento que está buscando un registro en una base de datos en lugar de un archivo en un directorio en un sistema de archivos. ¿Prefieres ver
y luego en bucle (por ejemplo, en C #) sobre el cursor devuelto buscando ID = 100, o dejando que el subsistema consultable haga lo que pueda para encontrar exactamente lo que está buscando.
Debería pensar que la mayoría de nosotros elegiría correctamente dejar que el subsistema realice la consulta exacta de interés. La alternativa implica potencialmente numerosos viajes de ida y vuelta con el subsistema, pruebas de igualdad ineficientes y renunciar al uso de cualquier índice u otros aceleradores de búsqueda, que nos proporcionan tanto las bases de datos como los sistemas de archivos.
Sí, porque así es cómo funciona esa API en particular: no es realmente nuestra elección, ya que esta es una función de biblioteca. Si la comprobación, antes de la llamada, no ofrece ningún valor adicional: tenemos que usar el try / catch de todos modos, ya que (1) pueden ocurrir otros errores más allá de FileNotFound y (2) la condición de carrera.
fuente
El lector de flujo no tiene nada que ver con eso. El antipatrón surge debido a un claro conflicto de intenciones entre el
foreach
y elif
:¿Cuál es el propósito de la
foreach
?Supongo que su respuesta será algo así como: "Quiero ejecutar repetidamente un código en particular"
¿Cuántos archivos esperas procesar?
Dado que solo puede tener un nombre de archivo específico (incluida la extensión) en una carpeta en particular, esto prueba que su código está destinado a encontrar un archivo aplicable.
Esto también se confirma por el hecho de que está devolviendo un valor de inmediato. En realidad no te importa una segunda partida, incluso si existiera.
Hay situaciones en las que esto no es un antipatrón.
Directory.GetFiles(".", SearchOption.AllDirectories)
), entonces es posible encontrar más de un archivo con el mismo nombre de archivo (incluida la extensión)"Test_"
, o cada"*.zip"
archivo.Tenga en cuenta que ambos casos requerirían que realmente procese varias coincidencias y, por lo tanto, no devuelva un valor de inmediato.
Las excepciones son caras. Nunca deben usarse en lugar de una lógica de flujo adecuada. Las excepciones son, como su nombre indica, circunstancias excepcionales .
Por esa razón, no debes eliminar el
if
.Según esta respuesta en SoftwareEngineering.SE :
Como resumen rápido de por qué, en general, es un antipatrón:
Si necesita envolver esto en un intento / captura, depende en gran medida de su situación:
Nada es cuestión de "usarlo siempre". Para probar mi punto:
Entonces, ¿por qué no todos llevamos este equipo de seguridad en todo momento?
La respuesta simple es porque hay inconvenientes en usarlos:
Ahora estamos llegando a algún lado: hay ventajas y desventajas . En otras palabras, solo tiene sentido usar este equipo en los casos en que los profesionales superan a los contras.
¿Debería terminar la llamada en un intento / captura? Eso depende en gran medida de si los beneficios de hacerlo superan el costo de implementarlo.
Tenga en cuenta que otros pueden argumentar que solo se necesitan unas pocas teclas para envolverlo, por lo que obviamente debe hacerse. Pero ese no es el argumento completo:
Entonces la elección es tuya. ¿Hay algún beneficio en hacerlo? ¿Crees que mejora la aplicación, más que un esfuerzo costoso para implementarla?
fuente