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!.

Anuncios

Responder

Introduce tus datos o haz clic en un icono para iniciar sesión:

Logo de WordPress.com

Estás comentando usando tu cuenta de WordPress.com. Cerrar sesión /  Cambiar )

Google photo

Estás comentando usando tu cuenta de Google. Cerrar sesión /  Cambiar )

Imagen de Twitter

Estás comentando usando tu cuenta de Twitter. Cerrar sesión /  Cambiar )

Foto de Facebook

Estás comentando usando tu cuenta de Facebook. Cerrar sesión /  Cambiar )

Conectando a %s