Cómo manejar el olor trivial de "duplicación de código" en Rails/Ruby

Question

Por lo tanto, todos nos esforzamos por reducir la duplicación (DRY) y otros olores, y mantener nuestro código lo más limpio y agradable posible. Para el código de Ruby, hay muchos instrumentos para detectar olores, por ejemplo, el bastante agradable servicio Caliber.

Sin embargo, parece que tengo una definición diferente de duplicación de código que las herramientas. Creo que esto podría estar relacionado con la forma de hacer las cosas de Ruby, donde casi nunca se accede directamente a una variable, sino a través de una llamada a un método. Considérese este fragmento de un controlador Rails:

def update_site_settings 
    SiteSettings.site_name = params[:site_name] 
    SiteSettings.site_theme = params[:site_theme] 
    expire_fragment('layout_header') 
    flash[:notice] = t(:Site_settings_updated) 
    redirect_to :controller => 'application', :action => 'edit_site_settings' 
end 

Esta se encuentra en posición con una advertencia duplicación de código, a causa de las dos llamadas al método "params". Entonces mi pregunta es, ¿realmente sería una mejora asignar el params a una variable local? Considero que la forma en que está escrito es la forma más clara y concisa de hacerlo, y el hecho de que params es un método y no una variable es simple "el costo de hacer negocios" en Ruby.

¿Estoy viendo esto de la manera incorrecta?

EDIT: En este caso, una forma más bonita podría ser la de hacer una actualización SiteSettings.update_attributes(params) estilo. Considere, si se quiere, el mismo problema en otro fragmento:

def update 
    @mailing_list = MailingList.find(params[:id]) 

    if @mailing_list.update_attributes(params[:mailing_list]) 
    flash[:notice] = t:Mailing_list_updated 
    redirect_to(mailing_lists_path) 
    ... 

Answer 1

Supongo que también podría declarar una instancia menor del código Feature Envy, aunque es algo trivial.

Tal vez un método de clase en MailingList podrían introducirse, por lo que el método de control se convierte en algo así como

def update 
    if @mailing_list = MailingList.update_attributes_by_id(params) 
    ... 

class MailingList 
    def self.update_attributes_by_id(params) 
    id = params.delete(:id) 
    find(id).update_attributes(params) 
    ... 

(no probado, lo que debe manipularse con cuidado)

me molestaría en la vida real? Probablemente no, por un lado la cosa de encontrar/actualizar en dos partes es tan común que las personas lo entienden de inmediato: alguien que ingresa al código como el anterior tendrá que detenerse y pensar un poco, incluso si tiene un nombre perfectamente expresivo.

Estos analizadores (ejecuto Kevin Rutherford's reek en mi propio código) son geniales, pero no entienden el contexto, por lo que rara vez van a ofrecer información perfecta. Son útiles para identificar áreas que pueden beneficiarse de la atención, pero habrá muchos falsos positivos y también extrañarán cosas, por lo que deben usarse con conocimiento.

Answer 2

Una cosa para recordar acerca de los conceptos de seco y olores de código es que son directrices. Están ahí para ayudarlo a pensar acerca de cómo organizar y simplificar su código en un sentido más del bosque para los árboles. Si bien es bueno tener siempre en cuenta estos conceptos, la repetición de código malicioso en un nivel tan pequeño a menudo terminará introduciendo complejidad innecesaria u oscuridad en el código. La comprensibilidad de su código también debe ser importante, y como usted dice, a veces el código que es más claro y conciso no es el mismo que el código donde se elimina cada rastro de repetición.