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.

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

Panda, Tiger, and Eagle code. #20

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
6 changes: 5 additions & 1 deletion lib/api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,11 @@ 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)
full_json = get_url_as_json(url)

return :NotFound if full_json.fetch("total").to_i == 0

struct = OpenStruct.new(full_json.fetch("movies").first)
Movie.new(id: struct.id.to_i,
title: struct.title,
year: struct.year,
Expand Down
69 changes: 69 additions & 0 deletions lib/collection.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
class Collection
Copy link
Author

Choose a reason for hiding this comment

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

I didn't realize until the end that I didn't have require statements in there for the Movie, although I'm clearly using Movie properties. How is that working? Is it just that the code that calls Collection objects has the require statements, so it connects properly or is Ruby doing something under the hood doing delayed evaluation?

Copy link
Member

Choose a reason for hiding this comment

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

Rails itself will do code auto-loading, but Ruby is not so forgiving.

If you were to open lib/collection.rb by itself, it wouldn't work.. but the entry point (movie_json.rb) has:

require_relative "lib/movie"
require_relative "lib/api"
require_relative "lib/collection"

so, it's available thereafter in the global space for use.

Copy link
Member

Choose a reason for hiding this comment

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

Well, that's not exactly true -- I looked at the Collection file, and it's really using anything Movie specific. That's a bit weird, right? but, the collection is passed things -- it could be a movie, or a coin, or a french fry.

Then, you call methods on it , like year, It simply sends the "year" method to the object in the array, and hope it responds_to it. If it does respond_to it, you'll get a value from it. If not, you'll get a no-method error.

That's the "REAL" reason you didn't need to require move.

Copy link
Member

Choose a reason for hiding this comment

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

Recommendation: since this collection is highly specific to movies, I'd name it "MovieCollection"

attr_reader :movies

def initialize
@movies = []
end

def add_movie movie
@movies << movie unless movie == :NotFound
end

def average_score
average_for_movies @movies
end

def average_for_movies(movie_list)
return 0 if movie_list.size == 0

movie_list.inject(0.0) {|sum, movie| sum + movie.score} / movie_list.size
end

def average_year
return :NoData if @movies.size == 0

@movies.inject(0) {|sum, movie| sum + movie.year} / @movies.size
end

def average_per_year
dictionary = {}
separate_by_years.each {|year_list| dictionary[year_list.first.year.to_s] = average_for_movies year_list}
dictionary
end

def separate_by_years
Copy link
Author

Choose a reason for hiding this comment

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

I feel like with all of the wiz-bang stuff in Ruby "decompose this array into multiple sub arrays based on a block's evaluation" should be easier, but I couldn't find out to do it without the recursion in separate_by_years and array_of_years. Any suggestions?

Copy link
Member

Choose a reason for hiding this comment

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

I think one of the reasons you're having trouble is that array feels too difficult here. I think hash is the appropriate data type here.

def movies_by_year(movies)
  hash = {}
  movies.each do |movie|
    hash[movie.year] ||= []
    hash[movie.year] << movie
  end
  hash
end

ok, so now you have a hash by year, which is easier to deal with.

In ruby, the data-type most often reached for is the hash. arrays are generally pretty stupid and simple lists.

Copy link
Author

Choose a reason for hiding this comment

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

Duh. That makes a lot more sense. Let me refactor this stuff and let you take another look. Thanks.

sorted_movies = @movies.sort { |x,y| x.year <=> y.year }
array_of_years([],[sorted_movies.first], sorted_movies.slice(1,sorted_movies.length - 1))
end

def array_of_years(years_array, current_movies, remaining_movies)
if remaining_movies.size == 0
years_array << current_movies
return years_array
else
movie = remaining_movies.first
if movie.year == current_movies.first.year
current_movies << movie
array_of_years(years_array, current_movies, remaining_movies.slice(1,remaining_movies.length- 1))
else
years_array << current_movies
array_of_years(years_array, [movie], remaining_movies.slice(1,remaining_movies.length- 1))
end
end
end

def rating_slope
yearly_averages = average_per_year
# I think the keys should already be sorted by year, but just in case do it anyway
sorted_years = yearly_averages.keys.sort

return :NeedMoreData if sorted_years.length < 2

first_avg = yearly_averages[sorted_years.first]
last_avg = yearly_averages[sorted_years.last]

(first_avg - last_avg).to_f / (sorted_years.first.to_i - sorted_years.last.to_i).to_f

end

end
51 changes: 48 additions & 3 deletions movie_json.rb
Original file line number Diff line number Diff line change
@@ -1,21 +1,66 @@
require_relative "lib/movie"
require_relative "lib/api"
require_relative "lib/collection"


@collection = Collection.new

def find_movie
puts "OH HAI. Search?"
puts "OH HAI. Add a movie to your collection?"
movie_title = gets
movie = Api.search_by_title(movie_title)
puts "Found: #{movie.title}. Score: #{movie.score}"
@collection.add_movie movie

if movie == :NotFound
puts "Didn't find that one."
else
puts "Found: #{movie.title}. Score: #{movie.score}"
end
end

def show_collection_average
puts "The average score of your collection is #{@collection.average_score}."
if @collection.average_score > 75
puts "Excellent tastes!"
elsif @collection.average_score > 25
puts "Nice collection."
else
puts "You have very unique tastes."
end
end

def show_year_average
puts "The average year of your collection is #{@collection.average_year}"
end

def show_averages_by_year
yearly_averages = @collection.average_per_year
puts "Your movie average by year is:"
yearly_averages.each { |year, avg| puts "#{year} : #{avg}" }
end

def show_slope
slope = @collection.rating_slope
puts "The slope of your movie ratings is #{slope}."
if slope < 0
puts "Your tastes are declining. You mad, bro?"
else
puts "The quality of films is improving. Release that anger!"
end
end

find_movie

while true do
puts "Search Again (Y/N)"
puts "Add another? (Y/N)"
answer = gets.upcase[0]
if answer == "Y"
find_movie
else
show_collection_average
show_year_average
show_averages_by_year
show_slope
break
end
end
14 changes: 14 additions & 0 deletions spec/api_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,18 @@
it "should return the year" do
movie.year.should eq(1994)
end

describe "empty movie tests" do

let(:emptyMovie) {Api.search_by_title("FHAHAHAHA")}

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

it "should return a not found sentintel when there is no movie" do
movie = Api.search_by_title("FHFHFHF")
movie.should eq(:NotFound)
end
end
end
96 changes: 96 additions & 0 deletions spec/collection_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
require_relative "../lib/movie"
require_relative "../lib/collection"

describe Collection do

let(:collection) {Collection.new}
let(:movie) {Movie.new(title: "Pulp Fiction", year: 1992, id: 233, score: 50) }
let(:movie2) { Movie.new(title: "Run Lola Run", year: 1992, id: 233, score: 98)}
let(:movie3) { Movie.new(title: "Run Lola Run", year: 1995, id: 233, score: 98)}

it "contains a set of movies" do
collection.movies.should eq([])
end

it "should add a movie to the collection" do
collection.add_movie movie
collection.movies.should eq([movie])
end

it "should not add the Not Found sentinel" do
collection.add_movie :NotFound
collection.movies.should eq([])
end

it "the average score of an empty collection should be zero" do
collection.average_score.should eq(0)
end

it "should have an average score the same as the movie in a single collection" do

collection.add_movie movie
collection.average_score.should eq(50)
end

it "should provide the average score for collection" do
collection.add_movie movie
collection.add_movie movie2
collection.average_score.should eq(74)
end

it "should provide the average year for the collection" do
collection.add_movie movie
collection.add_movie movie2
collection.add_movie movie3
collection.average_year.should eq(1993)
end

it "should provide the average score per year" do
collection.add_movie movie
collection.add_movie movie3
collection.add_movie movie2
collection.average_per_year.should eq({"1992" => 74, "1995" => 98})
end

describe "decompose movie array into list of movies by year" do

it "should return a list of lists for a single movie" do
array_of_arrays = collection.array_of_years([], [movie], [])
array_of_arrays.should eq([[movie]])
end

it "should put two movies with different years into different lists" do

separate_list = collection.array_of_years([], [movie], [movie3])
separate_list.should eq([[movie], [movie3]])
end

it "should put two movies with the same year in the same list" do

separate_list = collection.array_of_years([], [movie], [movie2])
separate_list.should eq([[movie, movie2]])
end
end

it "should separate a collection of movies by year" do
collection.add_movie movie
collection.add_movie movie2
collection.add_movie movie3
separate_list = collection.separate_by_years

separate_list.should eq([[movie, movie2], [movie3]])
end

it "should calculate the slope of ratings from the first year to the last year of the collection" do
collection.add_movie movie
collection.add_movie movie2
collection.add_movie movie3
collection.rating_slope.should eq(8)
end

it "should return a sentintal if it can't compute the slope" do
collection.add_movie movie
collection.rating_slope.should eq(:NeedMoreData)
end

end
31 changes: 31 additions & 0 deletions spec/fixtures/none_found.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@






























{"total":0,"movies":[],"links":{"self":"http://api.rottentomatoes.com/api/public/v1.0/movies.json?q=FHAHAHAHA&page_limit=1&page=1"},"link_template":"http://api.rottentomatoes.com/api/public/v1.0/movies.json?q={search-term}&page_limit={results-per-page}&page={page-number}"}