Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

panda level #14

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

panda level #14

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a46fa90
working version of panda level; testing of raising error does not wor…
dramanat Mar 10, 2013
c588dce
return :no_movie_found when invalid movie title entered instead of ra…
dramanat Mar 11, 2013
0b4b214
fix test for invalid movie title
dramanat Mar 11, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 11 additions & 6 deletions lib/api.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,17 @@ class Api

def self.search_by_title(title)
url = "http://api.rottentomatoes.com/api/public/v1.0/movies.json?apikey=#{APIKEY}&q=#{URI.encode(title)}&page_limit=1"
struct = OpenStruct.new(get_url_as_json(url).fetch("movies").first)
Movie.new(id: struct.id.to_i,
title: struct.title,
year: struct.year,
score: struct.ratings["critics_score"]
)
begin
struct = OpenStruct.new(get_url_as_json(url).fetch("movies").first)
Movie.new(id: struct.id.to_i,
title: struct.title,
year: struct.year,
score: struct.ratings["critics_score"]
)
rescue
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you are rescuing the entire search_by_title method, where no matter what exception is raised, you're showing "no_movie_found". That's not REALLY the case though. For example, if the API is down or broken, you still want people to be able to find their "Caddyshack"

#raise ArgumentError, "invalid movie #{title.strip} given"
return :no_movie_found
end
end


Expand Down
8 changes: 6 additions & 2 deletions movie_json.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,14 @@
require_relative "lib/api"

def find_movie
puts "OH HAI. Search?"
puts "OH HI. Search?"
movie_title = gets
movie = Api.search_by_title(movie_title)
puts "Found: #{movie.title}. Score: #{movie.score}"
if movie == :no_movie_found
puts "Invalid movie title : #{movie_title.strip}"
else
puts "Found: #{movie.title}. Score: #{movie.score}"
end
end

find_movie
Expand Down
51 changes: 37 additions & 14 deletions spec/api_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,48 @@

describe Api do

let(:movie) { Api.search_by_title("Forrest Gump") }
context "valid movie title" do

before do
Api.stub(:get_url_as_json) { JSON.parse(File.read("spec/fixtures/forrest.json")) }
end
let(:movie) { Api.search_by_title("Forrest Gump") }

it "should search for movies" do
movie.title.should eq("Forrest Gump")
end
before do
expect {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not add the expect here in the before block. The before block is adding this test to every example.

I do think having this in the before makes sense, because you want every time it executes get_url_as_json it should return that fixture.

Api.stub(:get_url_as_json) { JSON.parse(File.read("spec/fixtures/forrest.json"))

Api.stub(:get_url_as_json) { JSON.parse(File.read("spec/fixtures/forrest.json")) }
}.to_not raise_error
end

it "should return the score" do
movie.score.should eq(71)
end
it "should search for movies" do
movie.title.should eq("Forrest Gump")
end

it "should return the score" do
movie.score.should eq(71)
end

it "should return the id" do
movie.id.should eq(10036)
end

it "should return the year" do
movie.year.should eq(1994)
end

it "should return the id" do
movie.id.should eq(10036)
end

it "should return the year" do
movie.year.should eq(1994)
context "invalid movie title" do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is how I'd write this. No problem with the way you did, but this might be more readable.

context "invalid movie title" do
  it "should return :no_movie_found" do
    Api.stub(:get_url_as_json) { JSON.parse(File.read("spec/fixtures/invalid.json"))
    Api.search_by_title("hgh").should eq(:no_movie_found)
  end
end

let(:movie) { Api.search_by_title("hgh") }

before do
expect {
Api.stub(:get_url_as_json) { JSON.parse(File.read("spec/fixtures/invalid.json")) }
}.to_not raise_error
end

it "should return :no_movie_found" do
movie.should eq(:no_movie_found)
end

end


end