321bbdb120165eebbaf37e781d4ec71b

I thought of creating a before_filter to load the @course instance variable in to the show action - but it failed on the final elsif so I decided to manually add it in both conditions.

def show
    
    if params[:course_id] && permitted_to?(:browse, :videos)
      @course = Course.find(params[:course_id])
      @video = @course.videos.find(:first) 
      add_crumb("Watching #{@video.title}", course_video_path(@course, @video))
      
    elsif params[:course_id]
      @course = Course.find(params[:course_id])
      @video = @course.videos.find(params[:id])
      add_crumb("Watching #{@video.title}", course_video_path(@course, @video))
   
    elsif !permitted_to?(:browse, :videos)
      @video = Video.find(params[:id])
      add_crumb("Watching #{@video.title}", video_path(@video))
      
    end

Refactorings

No refactoring yet !

D41d8cd98f00b204e9800998ecf8427e

Joe

March 25, 2009, March 25, 2009 22:20, permalink

1 rating. Login to rate! 
def find_course
  @course = params[:course_id] && Course.find(params[:course_id])
end

def show
    if @course
       if permitted_to?(:browse, :videos)
          @video = @course.videos.find(:first)
       else
           @video = @course.videos.find(params[:id])
       end
       add_crumb("Watching #{@video.title}", course_video_path(@course, @video))
     
     elsif !permitted_to?(:browse, :videos)
       @video = Video.find(params[:id])
       add_crumb("Watching #{@video.title}", video_path(@video))
     end
end
83cef5e0ca6b31e30b85bfa5f04d8cb0

Colin Curtin

March 26, 2009, March 26, 2009 02:40, permalink

1 rating. Login to rate! 
def show
  @course = Course.find_by_id(params[:course_id])
  
  if permitted_to?(:browse, :videos)
    @video = @course.videos.find(:first)
    video_path = course_video_path(@course, @video)
  else
    @video = Video.find(params[:id])
    video_path = video_path(@video)
  end
  
  add_crumb("Watching #{@video.title}", video_path)
end
A8d3f35baafdaea851914b17dae9e1fc

Adam

March 27, 2009, March 27, 2009 04:37, permalink

1 rating. Login to rate! 
class CoursesController < ApplicationController
  before_filter :set_course
  
  def show
    @video = scoped_video_class.find(permitted_video_id)
    add_crumb("Watching #{@video.title}", polymorphic_path([ @course, @video ].compact))
  end
  
  private
    def set_course
      @course = Course.find_by_id(params[:course_id])
    end
    
    def scoped_video_class
      @course ? @course.videos : Video
    end
    
    def permitted_video_id
      permitted_to?(:browse, :videos) ? :first : params[:id]
    end
end

Your refactoring





Format Copy from initial code

or Cancel