Stylistic exception and bang usage

Question

I'm not sure whether I should be throwing exceptions in the below model code (and rescuing in the controller - rescues not implemented below) or returning nil (in both or one of the exception cases). Also, should the model method be identify, or is identify proper?

My line of thinking is if the title is not set when identify is called, something is wrong or it is being used incorrectly. Regarding the second exception, if there are no results, then the movie in question can not be scraped, thus making all operations later (which depend on the scraped data) useless.

I guess this comes down to return nil vs throw exception - I have been reading quite a few arguments on this subject and to me it seems proper to throw in both cases, thoughts? Perhaps i'm missing a "rails way" of doing this too? I'm fairly new to Rails. If it matters, I am using Rails 3.2.

Movie model

validates :title, :presence => true

def identify
  raise "No Title" if self.title.nil?

  search_content = Nokogiri::HTML open("http://akas.imdb.com/find?q=#{CGI::escape(self.title)};s=tt")

  top_result = search_content.xpath("//div[@id='main']/p[1]/b/a").first

  raise "No Results" if top_result.nil?

  self.title = top_result.content

  self.imdb_id = top_result["href"].split("/")[2][2..-1].to_i

end

def populate_data
  identify if self.imdb_id.nil?

  # ...scrape imdb and save content to model attributes...
end

MovieController

def create
  @movie = Movie.new :title => params[:movie][:title]

  @movie.populate_data

  if @movie.save
    # ...the usual set flash, else recover kinda thing...

Answer 1

should the model method be identify! or is identify proper?

You should only end the method name with a bang (!) if the method modifies the object it is called on. Furthermore, only if there is also another method of the same name without the bang which does not modify the object. So no, you should not use a bang in your method name.

return nil vs throw exception

I would take a cue from either ActiveRecord#find or #find_by_*. The first will throw an exception when given an ID that doesn't exist. The later will return nil. Think of it as entering a URL in your browser as opposed to doing a Google search. With the first, it's expected that you know the exact URL, with the later, it's not and you should get some sort of result or lack of result. According to what you say, you should be going with throwing exceptions.

However, I would rethink how you're doing things. I would do this with initialize instead of having to explicitly call populate_data.