Refactoring de “Fat Methods” – Episodio 2

Continuando con esta serie de ejemplos de como refactorizar métodos de controladores vamos a seguir ahora con uno que cuando lo volví a ver apestaba feo feo.

El método en cuestión tiene por objetivo que el usuario actual se suscriba (es decir, anote) para jugar un partido determinado. Para eso el siguiente código es el que está siendo ejecutado actualmente :

  # app/controllers/matches_controllers.rb
  def subscribe
    flash[:notice] = "Ya formás parte del equipo!" and 
        redirect_to match_path(@match) and 
        return if @match.has_player? current_user

    flash[:error] = "El equipo está completo." 
      and redirect_to match_path(@match) and ]
      return if @match.available_places(params[:team].to_i)  current_user, :team => params[:team]

    redirect_to match_path(@match)
  end

Lo feo del problema se puede ver desde dos perspectivas : una que es una excusa y otra que es la razón de la desprolijidad. La excusa para este código tan horrible es que en caso de no poder agregar el jugador al equipo elegido tengo que mostrar un mensaje de error y redireccionar. Pero el redirect_to lo que hace es setear un header nada más, es decir que no hace un return auto-mágico, y de no hacerlo yo el segundo redirect_to (ubicado al final del código) haría que se lance una Exception por haber reenviado los headers.

El problema real es que estoy delegando en el controlador la responsabilidad de la clase Player para determinar si es válido crear una instancia para un partido y usuario dado.

Es por eso que lo primero que debemos hacer es usar los validators de Rails para esta tarea :

  # app/models/player.rb
  def validate
    errors.add_to_base("Ya formás parte del equipo!") if match.has_player? user
    errors.add_to_base("El equipo está completo.") if match.available_places(team) < = 0
  end

En este caso los errores no dependen de un atributo, por lo que usamos add_to_base para que los errores digan lo que queremos. Este método validate es llamado por Rails al crear o actualizar una instancia de Player, y si hay algún error nunca llega a la DB.

Habiendo quitado la validación del controlador ahora podemos escribirlo de una forma más prolija y entendible :

  # app/controllers/matches_controllers.rb
  def subscribe
    @player = @match.players.create :user => current_user, :team => params[:team].to_i

    flash[:notice] = @player.errors.full_messages.join(", ") if @player.errors.any?

    redirect_to match_path(@match)
  end

Lovely :). Se crea un Player para el Match, se prepara una alerta visual en caso de que haya algún error y luego se redirige a la página del partido. Si ven el antes y el después creo que nadie me va a negar de la mejora :).

Por hoy es todo, hasta la próxima entrega!.

Refactoring de “Fat Methods” – Episodio 1

Luego de escribir mucho código uno comienza a mirar a ver que hacen los de al lado, y al mirar se da cuenta de lo mal que uno escribe código cuando comienza con un nuevo lenguaje :). Algo de eso me pasa hoy en día con ¡Falta Uno!.

En el camino de emprolijar las cosas para liberar el código tuve la necesita de poner a refactorizar el código porque hay partes que no son nada bonito. Voy a tratar de plasmar en los siguientes posts algunas lecciones aprendidas junto con el paso a paso del refactoring del código.

Antes que nada y como buena práctica es ideal tener buenos test sobre los que se va a refactorizar ya que es muy fácil romper alguna funcionalidad que se suponía que andaba. Si bien para los ejemplos acá voy a ignorar esta buena práctica, no se los aconsejo :).

Vamos a empezar con el método create de controlador Matches, encargado de crear un nuevo partido :

  def create
    @match = current_user.matches.create(params[:match])
    if @match.errors.empty?
      Player.create :match => @match, :user => current_user

      @notifications = @match.owner.friends.select {|f| f.notify_new_matches? }.collect(&:email)
      Emailer.deliver_match_created(@match, @notifications) if @notifications.any?

      flash[:success] = "El partido fue creado."
      redirect_to matches_path
    else
      render :action => 'new'
    end
  rescue
    render :action => 'new'
  end

Asusta un poco, ¿no?. Para entender el problema, veamos primero como que involucra crear un nuevo partido :

  • Crear el partido
  • Anotar al usuario que creó el partido en uno de los equipos
  • Notificar a todos los amigos que se creó un nuevo partido

Dado esto, pasemos a analizar los problemas. La primer pregunta que habría que hacerse es : ¿Quién es el responsable de anotar al jugador?, ¿Tiene sentido que un usuario pueda crear un partido sin que esté anotado?. En el contexto nuestro, la respuesta a la última pregunta es “No”, solo porque así lo defino yo :). Entonces podemos responder a la primer pregunta : El Match debería ser el responsable de anotar al dueño del partido cuando es creado.

Para lograr esto último lo que debemos hacer es mover la lógica de creación del Player dentro de un callback en el modelo Match, quedando algo así :

class Match  self, :user => owner
  end
end

De esta forma nos aseguramos que se cumpla lo pedido sin correr el riesgo de que se puedan crear matches sin que el owner este anotado inicialmente. También ganaríamos en el caso de que exista alguna otra forma de crear un partido, por ejemplo si agregáramos la posibilidad de crear un partido enviando un email, no tendríamos la necesidad de repetir la lógica inicial en dos lugares diferentes.

Y ahora nuestro controlador queda algo cómo :

  def create
    @match = current_user.matches.create(params[:match])
    if @match.errors.empty?
      @notifications = @match.owner.friends.select {|f| f.notify_new_matches? }.collect(&:email)
      Emailer.deliver_match_created(@match, @notifications) if @notifications.any?

      flash[:success] = "El partido fue creado."
      redirect_to matches_path
    else
      render :action => 'new'
    end
  rescue
    render :action => 'new'
  end

Lo siguiente es limpiar un poco lo lógica del modelo. Una cosa que molesta a la vista son las dos llamadas a render, ¿son realmente necesarias? La respuesta en NO. Con el rescue lo que hacemos es que dada una Exception vuelva al formulario de creación, lo mismo que si el partido no es válido, por lo que simplemente podríamos haber usado el método create! que lanza una Exception si el modelo no es válido evitándonos así el molesto if. Quedando ahora :

  def create
    @match = current_user.matches.create!(params[:match])

    @notifications = @match.owner.friends.select {|f| f.notify_new_matches? }.collect(&:email)
    Emailer.deliver_match_created(@match, @notifications) if @notifications.any?

    flash[:success] = "El partido fue creado."
    redirect_to matches_path
  rescue
    render :action => 'new'
  end

Que si lo comparan con la versión inicial es mucho más legible. “Fat model, skinny controllers” es la clave a seguir. Todavía queda por mejorar la forma en que se genera la lista de emails de los amigos que desean recibir la notificación, pero no es algo que me moleste de momento.

En otras entregas seguiré mostrando el resto del refactoring de otro métodos que son aún peores que este :).

Simple Forum plugin for Rails

La semana pasada tenía la necesidad de agregar un foro super simple para un cliente, basicamente querían que los usuarios del sitio puedan dejar comentarios de la campaña, hablen entre ellos y no mucho más. Ya tenía uno hecho todo mezclado en otra aplicación por lo que decidí que era hora de reescribir menos :).

Lo primero que se me pasó por la mente “algo ya debe haber hecho” y si bien es cierto, lo poco que encontré se apoya en Engines lo cual para mi no es aceptable. Simplemente no me gusta Engines :). La mejor opción tal vez es Beast pero integrarlo con otra aplicación no es simple y tener el foro de forma externa no era una opción.

Por lo que decidí empezar un “site project” a partir del código que ya tenía, que al haberlo escrito hace mucho era horrible por lo que el resultado fue casi casi que codeado desde 0.

La idea es que en 3 pasos uno pueda tener el foro andando integrado en el sitio, y creo que mas o menos se logra con algunos “problemas”. El principal tema es que al estar los controllers en el plugin, y estos no se recargan en development, es medio molesto usarlo porque depende de que se haga hay que reiniciar el server a cada rato (tira unos stack level too deph”).

Acá está el código para el que quiera probarlo, testearlo y recomendar alguna alternativa. Sobre todo si encuentran errores de gramática/sintaxis en la poca documentación que hice se agradecería :).

Multiupload de imágenes con Prototype

Desde hace unos días que estoy haciendo un widget que soporte upload de múltiples archivos para una aplicación web. No fue fácil el comienzo pero despues de varias horas (unas 8 hasta este momento) ya va tomando forma.

Para poder trackear el upload de cada archivo utilizo apache_mod_upload_progress, un genio “Drogomir” :). Para compilarlo en OSX tuve algunos problemas ya que apache2 esta compilado en x86_64 y el default del apsx es x86 pero googleando se encuentra fácil como pasarle el parámetro al gcc. Lo otro que necesitamos tener instalado es mod_rails y apache 2.2. Cuando termine el código y lo publique estará todo explicado en detalle :).

El segundo problema grande fue el formulario. Para hacer el upload lo que hago es crear un iframe oculto y cambiar el target del formulario a ese frame (de esta menera si no tenemos javascript la aplicación degrada automáticamente al upload de imagenes individuales y el usuario no se entera), pero claro, necesitaba tener múltiples input:file, uno por cada archivo a subir. De ponerlos todos juntos tendríamos un POST super gigante que no era lo que se buscaba ya que no podría trackear cada upload por separado.

La solución fue, cada vez que se selecciona un archivo sacar el INPUT del form y guardarlo en un array. El espacio vacío se reemplaza con un nuevo INPUT y como todo es tán rápido, uno no se da cuenta. El problema llegó cuando terminaba el primer archivo, tenía que volver a agregar el siguiente file al formulario y hacer otra vez el submit. Pero si uno llama $(form).submit() desde javascript, el callback onSubmit no es ejecutado (defecto de las implementaciones de todos los navegadores que probé y parece que no va a cambiar) por lo que no era útil.

La solución finalmente fue simular el click enel botón enviar con un simple $(submit_button).click() que resuelve el problema anterior. Les dejo el video para que lo disfruten :).

httpv://www.youtube.com/watch?v=7PqZg_1Pi1w

has_sitemap plugin

Este fin de semana tenía que agregar a 4 sitios de clientes un sitemap para mejorar la indexación por los motores de búsqueda, así que para hacerlo una vez y rápido, decidí aprender a hacer plugins para Rails.

No es que sea una magia absoluta, de hecho es muy tonto hacer uno. Pero cómo codeas adentro del plugin creo que es importante, porque la gente los usa como cajas negras, y si es un desastre, todo lo de afuera se vuelve un desastre.

El plugin permite agregar sitemaps tal cual lo define el protocolo, mediante un simple helper y una función adicional donde el controller debe generar la data que desea agregar. Lo que se encapsula es básicamente el generador del XML.

Todo se genera dinámicamente y por ahora solo es útil para sitios realmente chicos (< 100 urls) como los que yo necesitaba. El protocolo especifica que se pueden poner hasta 50k de URLs (o 10Mb) por lo que este plugin no es útil, al menos por ahora, para esos casos.

El código está hosteado en http://github.com/Gazer/has_sitemap/tree/master.

Oregano cambia a Git

Hoy terminé de decidir que mover el main repo de Oregano a Git era una buena idea, principalmente porque ya me estaba cansando de hacer branches duplicando directorios :P.

A partir de ahora el source principal estará en GitHub, para todo aquel que quiera hacer un fork o simplemente pullear su propio tree local.

Para migrar el repo desde darcs a git usé la opción que explican acá, un script en ruby que funcionó de los más bien. Trate de usar uno en python que también nombra, pero tenía que instalar algunos módulos y con ruby nada, so, ganó el que menos laburo me dió :).

Lo primero que van a venir a llorar es, de cajón, “¿por qué en GitHub que es privativo? ¿por qué no Gitorious?” o cosas así. La verdad no tengo ninguna razón técnica, ni problemas con ninguno de los servicios. Simplemente en GitHub ya tenía cuenta, ya tenía mi llave pública configurada y ya conocía donde hacer click, so, fue la elección más cómoda simplemente :).

En fin, voy a ir actualizando la documentación, wiki y demás a medida que me acuerde :D.

¡Falta Uno!

Hace un par de años, enojado por lo difícil que era organizar un partido de fútbol con mis amigos, me puse a desarrollar una paginita web pedorra en PHP para organizar partidos. Obviamente, como muchas de las cosas que empiezo, quedó en la nada. Si bien funcionaba, no me calente en ponerlo online ni comentarlo con mis amigos.

Hace dos semanas la idea reflotó, principalmente porque quería sumar experiencia en Ruby On Rails, por lo que decidí subir la apuesta. De eso se trata ¡Falta Uno!.

La idea es simple : te registrar, te relacionas con tus amigos, creas un partido y despues se van anotando. En la página se puede ver quiénes se anotaron y un pequeño board para postear mensajes por si acaso (vale putear, organizar quién juega con quién, etc), así como los datos de la cancha, hora y demás.

Ahora bien, no hace falta que todos los que van a jugar tengan cuenta. ¿Por qué? Simplemente porque si no la cosa no funcionaría :).

Con mi grupo de amigos suele pasar lo siguiente : 3 horas antes de que empiece el partido llega un mail diciendo “me bajo porque me duele la uña”, y le sigue un mail con “uhhh, che, falta uno, quien consigue ??”.

Este sujeto que aparece a último momento, es muy probable que sea el primo del tio de un amigo de un amigo, y casi seguro que no tendría cuenta y mucho menos relación de amistad con el creador de un partido (para poder anotarte a un partido tenés que ser amigo del creador por ahora).

Entonces?, bien, el sistema permite anotar a un “guest player“, poniendo solo el nombre y ocupando la plaza y así se mantiene simple la creación del partido.

Otro feature que se agrega, que es totalmente opcional y lo hice porque me gusta tenerlo en cuenta, es poder votar como jugaron tus amigos en ese partido. Hay 5 categorías para puntuar entre 1 y 5, y luego entre todos los votos se calcula el promedio por partido, y también un promedio general por usuario.

Este último feature es totalmente opcional y puede ser omitido si no tenés ni tiempo ni ganas de usarlo :).

En fin, el site no creo que tenga mucho éxito, si lo tiene, groso :), pero por lo menos mis amigos y yo vamos a dejar de putear para organizar un picadito.

Algunos datos “técnicos” sobre lo usado :

  • Ruby On Rails 2.0.2
  • Plugins
    • attachment_fu : Para upload de avatars
    • has_many_friends : Relaciones de amistad entre usuarios
    • restful_authentication : No puede faltasi hay un login 🙂
    • will_paginate : Tengo que migrar a usar la versión en gema.
    • calendar_helper : Para armar el calendario de partidos
    • simple_format : Para formatear el texto plano de los comentarios.
  • Server : mongrel/ apache (mod_balancer+ mod_rewrite) / mysql