views:

92

answers:

2

I know, if i will write "redirect_to" instead of "render :action", i'll lose my @object's errors.

My code:

  def play
    @room = params[:id][0,1]
    @current_players = CurrentPlayer.all(:conditions => {:room => @room})
  end

  def join
    @new_player = CurrentPlayer.new(:user_id => current_user.id, :team_id => nil, :room => params[:room], :mode => params[:mode], :ip => request.remote_ip)
    if @new_player.save
      @game = create_game(params[:room], params[:mode])
      if @game
        flash[:notice] = 'Game created'
        return (redirect_to :action => "game_details", :recent => true)
      else
        flash[:error] = 'Game not created'
      end
   else
     return render(:action => 'play')
   end
    redirect_to_back
  end 

If a user will click a "join" link in play.html.erb, Rails will send request to "join" action, then, if there were errors, i need to show them to user.

But i can't write just redirect_to, and my code is now:

  def join
    @new_player = CurrentPlayer.new(:user_id => current_user.id, :team_id => nil, :room => params[:room], :mode => params[:mode], :ip => request.remote_ip)
    if @new_player.save
      @game = create_game(params[:room], params[:mode])
      if @game
        flash[:notice] = 'Game created'
        return (redirect_to :action => "game_details", :recent => true)
      else
        flash[:error] = 'Game not created'
      end
   else
    # == Very ugly and not DRY (just copypaste from 'play' action) ==
    @room = params[:id][0,1]
    @current_players = CurrentPlayer.all(:conditions => {:room => @room})
    # ===
     return render(:action => 'play')
   end
    redirect_to_back
  end 

How can i avoid that code?

+1  A: 

Just refactor the common code into a private method. I call the new method manually here, but you could also call it via a before_filter for those two actions.

def play
  setup_play
end

def join
  @new_player = CurrentPlayer.new(:user_id => current_user.id, :team_id => nil, :room => params[:room], :mode => params[:mode], :ip => request.remote_ip)
  if @new_player.save
    @game = create_game(params[:room], params[:mode])
    if @game
      flash[:notice] = 'Game created'
      return (redirect_to :action => "game_details", :recent => true)
    else
      flash[:error] = 'Game not created'
    end
  else
    setup_play
    return render(:action => 'play')
  end
  redirect_to_back
end

private

def setup_play
  @room = params[:id][0,1]
  @current_players = CurrentPlayer.all(:conditions => {:room => @room})
end
Jimmy Cuadra
+1  A: 

Just reuse your play action, since the parameters are the same. Also, I suggest the following to avoid so many nested if statements

def play
  @room = params[:id][0,1]
  @current_players = CurrentPlayer.all(:conditions => {:room => @room})
end

def join
  @new_player = CurrentPlayer.new(:user_id => current_user.id, :team_id => nil, :room => params[:room], :mode => params[:mode], :ip => request.remote_ip)
  if ! @new_player.save
    play
    return render(:action => 'play')
  end

  @game = create_game(params[:room], params[:mode])
  if ! @game
    flash[:error] = 'Game not created'
    return(redirect_to_back)
  end

  flash[:notice] = 'Game created'
  return redirect_to(:action => "game_details", :recent => true)
end 
Tim Harper
jimmy's suggestion to extract the setup logic is good.
Tim Harper
thanks, i just added 'play' and it works now =). Only one notice - I prefer to use "unless" instead of "if !"
FancyDancy