Control de flujo de Ruby: ¿lanza una excepción, devuelve nil o deja que falle?

Question

Estoy pensando en las mejores prácticas de control de flujo. ¿A qué camino debería ir?

1) NO MARCA nada y dejar que el programa falle (código más limpio, mensajes de error naturales):

def self.fetch(feed_id) 
    feed = Feed.find(feed_id) 
    feed.fetch 
    end 

2) fallan sin devolviendo nula (sin embargo, "código limpio", dice, que se nunca debe devolver null):

def self.fetch(feed_id) 
    return unless feed_id 
    feed = Feed.find(feed_id) 
    return unless feed 
    feed.fetch 
    end 

3) lanzar excepciones (porque es excepcional no encontrar un pienso, id):

def self.fetch(feed_id) 
    raise ArgumentError.new unless feed_id 
    feed = Feed.find(feed_id) 
    raise ArgumentError.new unless feed 
    feed.fetch 
    end 

Para decirlo en otras palabras: ¿debería estar usando las condiciones de protección activamente, o es mejor confiar en los métodos de Ruby/Rails y dejarlos lanzar una excepción, si ocurre algo incorrecto?

Answer 1

1) No comprobar nada y dejar que el programa falle (código más limpio, mensajes de error naturales):

Está bien "dejar que el programa falle" con excepciones conocidas y documentadas, pero obtener NoMethodError desagradable porque trataste de usar un objeto nil es simplemente descuidado. En el ejemplo particular, ActiveRecord#find plantea una documentada ActiveRecord::RecordNotFound excepción, por lo que la OMI este es el camino a seguir:

def self.fetch(feed_id) 
    Feed.find(feed_id).fetch 
end 

2) fallan sin devolviendo nula (sin embargo, "código limpio", dice, que que debiera nunca devuelva nulo):

Eso está bien como consejo general, pero Ruby está atestado de métodos que devuelven nil; y eso está bien (de nuevo, siempre y cuando esté documentado), simplemente significa "Nada" (y permite el patrón muy compacto something_that_can_be_nil || another_value). En este caso me gustaría escribir usando concisa de Ick maybe:

def self.fetch(feed_id) 
    Feed.find_by_id(feed_id).maybe.fetch 
end 

3) lanzar excepciones (porque es excepcional no encontrar un pienso, id):

Sí, pero entonces deje que el método genere la bien conocida excepción RecordNotFound, no una excepción personalizada (a menos que desee abstraer el hecho de que está trabajando con AR, lo que puede ser muy engorroso).

Answer 2

Creo que la respuesta correcta es: depende. El usuario teóricamente nunca debería encontrar ningún mensaje de error del marco. Siempre debe estar preparado para manejar estas excepciones. La elección es suya (si no es una interfaz usada externamente o algo así).

Si toma la primera ruta, creo que debe pujar la consulta de que existe algún feed con esa identificación, y luego intente recuperarla. Si el feed desaparece entre los dos, puede ser un problema real para informar. El tercero es básicamente lo mismo. Debes asegurarte de que manejaste todas las situaciones y de que esa excepción de lanzamiento puede ayudar a evitar que el usuario vea el error.

La segunda solución es básicamente esto, pero con un manejo interno. Con el nil indica que hay algún problema. También debe manejarse, informarse al usuario, o algo. El inconveniente es que si olvida esto, puede engañar al usuario.

Usaría el primer método, con una comprobación adicional antes para asegurarme de que existe. Pero depende del uso.

Answer 3

Iría por la versión limpia.

Si no proporciona un feed_id al método fetch, ruby ​​aumentará ArgumentError: wrong number of arguments(0 for 1), por lo que la primera parte del n. ° 3 no tiene sentido.

Si no se proporciona un válido feed_id, entonces la llamada Feed.find(feed_id) lanzará una expresión diferente, muy probablemente ActiveRecord::RecordNotFound con un mensaje ya sea diciendo que no podía encontrar un pienso con el ID suministrado, o si no hay identificación fue suministrada (el parámetro feed_id era nil), que no pudo encontrar un feed sin una ID.

Para mí, me parece un poco estúpido llamar al método con feed_id = nil, por lo que probablemente argumentaría que "si envía una entrada no válida, podría romperse", y en ese caso, creo que el ActiveRecord::RecordNotFound le dará mucho más información sobre qué salió mal que si levanta un ArgumentError.

La devolución de un valor nulo rara vez es algo bueno, ya que eso no le dirá qué salió mal en realidad. Por lo tanto, también descartaría el n. ° 2.