CODEWITHSUNDEEP
ruby-on-railsruby
tsubasa
tsubasa

Reputation: 31

How should I write code if a params[:id] doesn't exist in the db

I'm writing a show action in a controller and want to branch if no id received from params[:id] exists in my database.

My app is still being created. I've tried conditional branching and exception handling. However, they didn't work as expected.

class ArticlesController < ApplicationController

  #...

  def show
    begin
      @article = Article.find(params[:id])
    rescue
      render root
    end
  end

  #...

end

I expect to be redirect to the root if any error occurs.

The above code returns the record if it is found successfully. However, an ActiveRecord::RecordNotFound in ArticlesController#show occurs if no record is found.

How should I write code for this error?

Upvotes: 0

Views: 77

Answers (5)

Ashok Damaniya
Ashok Damaniya

Reputation: 303

why write so much code, while you can achieve with two lines only.

class ArticlesController < ApplicationController
  def show
    @article = Article.find_by(id: params[:id])
    redirect_to root_path unless @article.present?
  end 
end

Upvotes: 0

Evan
Evan

Reputation: 571

Maybe You can code like this:

 def show
    @article = Article.find_by(id: params[:id])
    unless @article.present?
      flash[:error] = "Not Found."
      redirect_to root_path
    end
  end

Upvotes: 0

B&#249;i Nhật Duy
B&#249;i Nhật Duy

Reputation: 322

You can handle the exception from ArticlesController but I advise put the code at ApplicationController like this:

rescue_from ActiveRecord::RecordNotFound do
   redirect_back fallback_location: root_path, alert: t("resource_not_found")
end

Upvotes: 0

jvillian
jvillian

Reputation: 20263

How should I write code for this error?

The short answer is "you shouldn't".

Exceptions should, IMO, be exceptional. It sounds like you might expect this circumstance, so I suggest you write code that handles the scenario without raising an exception.

If you really think you'll have circumstances where the Article record does not exist for the given params[:id] (seems a little odd, but I guess it's possible), then I would suggest that you use .find_by instead of .find:

class ArticlesController < ApplicationController

  #...

  def show
    if @article = Article.find_by(id: params[:id])
      # do something with the article 
    else
      render root
    end
  end

  #...

end

It seems like to you might want to do a redirect instead of a render:

class ArticlesController < ApplicationController

  #...

  def show
    if @article = Article.find_by(id: params[:id])
      # do something with the article 
    else
      redirect_to root_path
    end
  end

  #...

end

But, maybe not...

Also, rescuing everything like this: 

def show
  begin
    @article = Article.find(params[:id])
  rescue
    render root
  end
end

...is generally viewed as code smell. There are a lot of articles on the interwebs about why.

Upvotes: 2

Vibol
Vibol

Reputation: 1168

Your code should be written like this:

 def show
    begin
      @article = Article.find(params[:id])
    rescue
      redirect_to root_path
    end
 end

Upvotes: -1

Related Questions