miércoles, 15 de diciembre de 2010

Diseño Simple

Hoy no he podido hacer pair programming con ningún "edenite" :( Los que no estaban en proyectos, estaban liándola parda organizando cosillas para Madrid :) Preparaos para la que se nos viene encima :D Ya se están empezando a mover cosas y nos vamos a divertir mucho :D Si queréis estar informados de todo lo que se organice para Eden Madrid debéis seguir esta cuenta de twitter.

Pero a lo que iba, que me desvío :P Hoy me ha tocado enfrentarme a mi código solo (aunque Chris me ha ayudado con una cosilla que se me resistía :D ) y me ha surgido una duda que, creo :P, las 4 reglas del diseño simple han conseguido aclarar.

Las 4 reglas del diseño simple

  1. Runs all the tests.
  2. Expresses every idea that we need to express.
  3. Says everything OnceAndOnlyOnce.
  4. Has no superfluous parts.

  1. Pasan todos los tests.
  2. Expresa cada idea que necesitamos expresar.
  3. Dice lo que dice una única vez.
  4. No tiene partes superfluas.

El orden de las reglas define su importancia, por lo que, si dos reglas entran en conflicto, debemos quedarnos con la primera de las dos.

El problema
Tengo una clase que debe dar de alta en el entorno un par de variables. Ambas variables dependen de si existe una cookie con el idioma en dicho entorno, por lo que el mismo if puede valernos para dar de alta las dos variables, tal que así:



Sin embargo, el set_variables(env) del primer método no me parece lo suficientemente expresivo, así que lo he cambiado por esto:



Está claro que, en este último caso, tenemos duplicación (Nos cargamos la tercera regla), pero aumenta la legibilidad (la segunda regla). He estado dudando si dejar la duplicación o eliminarla y, al final, he decidido que me gustaba más la legibilidad del segundo caso. No estoy seguro de si es lo correcto, veremos mañana cuando programe con un "edenite" al lado :D A vosotros, ¿qué os parece?

Un saludo.

PD: Spencer me ha pasado hoy este enlace sobre tipografía en la web. No lo he podido leer todavía, pero parece muy molón :D

5 comentarios:

  1. Hola.

    No sé nada de ruby, así que perdona si la lío.

    A mí me gusta más algo así:


    def set_variables(env)
    if has_language_cookie?(env)
    set_variables_from_previous_cookie(env);
    else
    set_variables_with_new_cookie(env);
    end
    end

    El resto ya te lo imaginas ;)

    Vamos, es sólo mi opinión...

    ResponderEliminar
  2. Pero set_variables no es muy descriptivo que digamos ¿no? Me pasa lo mismo con set_variables_from_previous_cookie... Para saber que variables me tengo que meter dentro de esos métodos. Por eso mi elección ha sido la otra, la de ser más expresivo en el método call.

    También te digo que, ahora mismo, no tengo clara la solución :D

    ResponderEliminar
  3. Ah, pensé que se sacaba por "el contexto" (la clase a la que pertenezca).

    La cuestión es que yo he tratado de agrupar operaciones similares (sólo llamadas a funciones). Ahora falta encontrarles un nombre adecuado. Personalmente me gusta que en el nombre se note que están relacionadas.

    ¿Set_language_environment_variables(), tal vez?

    (lo escribí anoche, pero no puse el captcha!!)

    ResponderEliminar
  4. Hola Alberto, creo que el problema que te impide avanzar es que estas siendo muy "procedural". Puedes definir objetos/clases que hagan acciones de configuración en el entorno. Una acción sería configurar la variable de lenguaje y otro el banner. No soy muy fluido con el Ruby pero esto te podría servir:

    def call(env)
    configure_env_for_language_with_actions(env, LanguageVariableConfigurationAction.new,BannerConfigurationAction.new);
    call_haml_serve(env) do |response, preferred_locale|
    set_preferred_locale_cookie(response, preferred_locale) if AVAILABLE_LOCALES.include?(preferred_locale)
    end
    end

    def configure_env_for_language_with_actions(env, *configurationActions)
    has_language_cookie=has_language_cookie?(env)
    configurationActions.each do | configAction |
    if has_language_cookie
    configAction.doConfigurationWithLanguageCookie(env)
    else
    configAction.doConfigurationWithoutLanguageCookie(env)
    end
    end
    end
    Y las clases que representan acciones de configuración:
    class LanguageVariableConfigurationAction
    def doConfigurationWithoutLanguageCookie(env)
    env[:locale] = user_language_from_cookie(env)
    end
    def doConfigurationWithoutLanguageCookie(env)
    browser_languages = BrowserLanguages.new(env['HTTP_ACCEPT_LANGUAGE'], AVAILABLE_LOCALES)
    env[:locale] = browser_languages.preferred_language
    end
    end
    class BannerConfigurationAction
    def doConfigurationWithLanguageCookie(env)
    env[:include_language_banner] = false
    end
    def doConfigurationWithoutLanguageCookie(env)
    browser_languages = BrowserLanguages.new(env['HTTP_ACCEPT_LANGUAGE'], AVAILABLE_LOCALES)
    env[:include_language_banner] = browser_languages.accepts_available_languages?
    end
    end
    Perdona los gazapillos de Ruby, como podrás notar por algunas cosas, el JAVA me acompaña. Seguro que con bloques de código se puede apañar algo mejor.
    Conociendo algo mejor el problema, podríamos decidir cosas más sutiles, como si es conveniente evitar la duplicación de la obtención de browser_languages (sólo si siempre se usa en el caso de no cookies) o si, como he hecho, realmente tenemos que evitar la duplicación del if (sólo si casi siempre la acción es diferente en función de si tenemos cookie o no).
    Esta solución te permite añadir más pasos (acciones) de configuración del entorno en función del lenguaje, sin tocar el método call y creo que es expresiva. Sin embargo, es más compleja y tal vez no te merece la pena si sabes que es poco probable que se añadan más acciones de configuración, y tienes muy seguro que sólo tienes esas dos.

    ResponderEliminar
  5. Hace poco tambien nos paso eso con un codigo nuestro. Al sacar factor comun y dejar un sólo método, había perdido semántica de negocio y despues de un tiempo resultaba dificil trabajar con el. Duplicamos parte del algoritmo pero ahora el código es mucho más facil de entender, y al final no hay ni tanta duplicidad :-)

    ResponderEliminar