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