@jwo Eagle level has so much Math...
Question:
"1. Tiger Level, plus display the average year for the ratings for the movie selected." - Are you asking to calculate the average rating for all movies in the searched year?
i.e. expand on the API methods

@jwo Some questions around RSpec before I continue with the Eagle level:

• When I run the rspec command in order to run multiple specs, some of the assertions in movie_spec.rb fail due to api_spec.rb executing first. How do you change scope of RSpec within a file as well as a test suite.
• How do you recreate this scenario using stub/mock objects?

describe ".average_rating" do
    it "should have no average rating initially" do

    end

• Can I do the above using my own stub/mock Movie objects where I have only set the @score?

When I run the rspec command in order to run multiple specs, some of the assertions in movie_spec.rb fail due to api_spec.rb executing first. How do you change scope of RSpec within a file as well as a test suite.

Generally this means your tests have side-effects, and generally should not happen. Is this happening now on your pull-request?

How do you recreate this scenario using stub/mock objects?

Seems you wouldn't need to -- you could just check the average-rating before calling anything and make sure it's 0, or that it throws an exception.

Can I do the above using my own stub/mock Movie objects where I have only set the @score?

Maybe? But usually you don't want to think about internal instance variables. It's a sign that you should pass in a data-object --- to separate your data from your data-methods.

@jwo Hmmm. I'm using Movie#build to keep track of the Movies searched.
You ask "Display the average movie Rating". Is this for both the searched Movies and for Movies relative to the current search?
Also, it seems RottenTomatoes protects its data by not revealing all the "blockbuster" movies via their API. As such, I'm getting Documentaries returned with a -1 critic rating....

I'm using Movie#build

I see you're storing the movies in a class variable. You may want to move this to another class, like "MovieLibrary"

it seems RottenTomatoes protects its data by not revealing all the "blockbuster" movies via their API. As such, I'm getting Documentaries returned with a -1 critic rating....

Maybe try older movies:

OH HAI. Search?
y
Found: 10 Things I Hate About You. Score: 61
Search Again (Y/N)
y
OH HAI. Search?
Groundhog Day
Found: Groundhog Day. Score: 97
Search Again (Y/N)
y
OH HAI. Search?
3 Days To Kill
Found: 3 Days To Kill. Score: -1
Search Again (Y/N)
y
OH HAI. Search?
Top Gun
Found: Top Gun. Score: 55
Search Again (Y/N)
y
OH HAI. Search?

@jwo Gotcha. Am I keeping track of just the searched movies in MovieLibrary?
Should I exclude the -1 rated movies?
Also, the API does not provide for a way to search for movies by year or any other field. What I'm doing is looping through the JSON request and pulling out the the entries from the "movies" key which has a sub-array.
Is this correct?
I'm thinking that my final output should read:

OH HAI. Search?
Top Gun
Found: Top Gun. Score: 55
Average Score for the year: 99
Search Again (Y/N)

Am I keeping track of just the searched movies in MovieLibrary?
yes

Is this correct?

yep, you have an excellent collection of searching capabilities in Ruby. The API should just give you some data to slice and dice.

your final output looks like a fine start

@jwo How do I create a mock Movie instance in order to check if it is cataloged in the libray?

describe "#register" do
    it 'should add a new movie to the library whenever a search happens' do
      movie = mock(:movie) # some API class call
      expect(MovieLibrary.all_movies).to include (movie)
    end
    xit 'should increase the movie count' do

    end
  end

Is there a rule of thumb regarding when to mock/stub?

you would stub Api.search_by_title to force it to return what you wanted

Is there a rule of thumb regarding when to mock/stub?

• You should stub out methods when you want to force a certain state in order to test it.
• You should send in a mock as a dependency to another colaborator.

@jwo You've kinda lost me.
New code is up. Any comments?
MovieLibrary#average_rating is returning NaN?

You're getting NaN because you have two repositories. Movies' class variable, and the MovieLibrary. Simplify that to 1 and your problems should go away.

@jwo Movie class looks a lot cleaner/readable.
NaN issue still persists when running movie_library_spec.rb

For the future, please include the full exception message -- it gives you a hell of a lot of information:

  1) MovieLibrary#average_rating should have no average rating when no movies are cataloged
     Failure/Error: movie_library.average_rating.should eq(0)

       expected: 0
            got: NaN

       (compared using ==)
     # ./spec/movie_library_spec.rb:33:in `block (3 levels) in <top (required)>'

What happens when you divide by 0 in real life?

@jwo Hmmmmm.
How do I add the DivisionByZero check to

  def average_rating
    result = all_movies.inject(0.0) { |sum, movie| sum + movie.score } / all_movies.size
  end

If you have a specification that it should return 0, then you'll need to pre-check:

  def average_rating
    return 0 if movie_count == 0
    all_movies.inject(:)/ all_movies.size
  end

@jwo Ditto on the full exception message.

@jwo The yearly average is calculating correctly but I feel there some refactoring needed below...
I can see the duplication but I'm unsure about how to move things about.

 def average_rating_by_year(year)
    result = all_movies.select {|movie| movie.year == year}
    return 0 if result.length == 0
    result.inject(0.0) { |sum, movie| sum + movie.score } / result.size
  end

  def average_rating
    return 0 if count == 0
    all_movies.inject(0.0) { |sum, movie| sum + movie.score } / all_movies.size
  end

Give it a try -- I'll give what I like after you try.

@jwo Refactored it to.

  def average_rating_by_year(year)
    result = all_movies.select {|movie| movie.year == year}
    return 0 if result.length == 0
    average_rating(result)
  end

  def average_rating(movies)
    return 0 if count == 0
    movies.inject(0.0) { |sum, movie| sum + movie.score } / movies.size
  end

Is the use result = bad style?

I think you could get away without it:

  def average_rating_by_year(year)
    average_rating all_movies.select {|movie| movie.year == year}
  end

  def average_rating(movies)
    return 0 if count == 0
    movies.inject(0.0) { |sum, movie| sum + movie.score } / movies.size
  end

@jwo Wow.

@jwo Slope calculation added as:

  def calculate_slope
    sorted_catalog = sort_by_year
    (sorted_catalog.first.score - sorted_catalog.last.score).to_f / (sorted_catalog.last.year - sorted_catalog.first.year).to_f
  end

  def sort_by_year
    all_movies.sort { |movie,another_movie| movie.year <=> another_movie.year }
  end

I feel sorted_catalog = sort_by_year is a code smell. Do you agree?

I think it could be an input to the method. Right now, calculate_slope has a dependency that another method exists named sort_by_year. It's not very usable by itself. So, as a general rule, try to pass in dependencies for methods like this.

  def calculate_slope(catalog)
    first = catalog.first
    last = catalog.last
    (first.score - last.score).to_f / (last.year - first.year).to_f
  end

  def sort_by_year (movies)
    movies.sort { |movie,another_movie| movie.year <=> another_movie.year }
  end

The only thing is that the slope should already be calculated when calling it.

@jwo Got it!
Made the MovieLibrary#catalog central to setting up the MovieLibrary instance.

  def catalog(movie)
    @movies << movie
    @library = sort_by_year(@movies)
    @slope = calculate_slope(@library)
    return @library
  end

@jwo Added a program summary.
Preventing NilClass errors when no movie is found.
Is this good Ruby?

unless movie.nil?
movie_library.catalog(movie)
puts "Found: #{movie.title}. Score: #{movie.score}"
end

And preventing NaN errors when calculating a slope for a single movie.

  def calculate_slope(catalog)
    first = catalog.first
    last = catalog.last
    return 0 if first == last
    (first.score - last.score).to_f / (last.year - first.year).to_f
  end

Is return 0 if first == last the best way to go about checking equaity in terms of working with the same object?

Is this good Ruby?

unless movie.nil?
  movie_library.catalog(movie)
  puts "Found: #{movie.title}. Score: #{movie.score}"
end

The intensions are good. However, I would invoke a rule where you only use "unless" as a 1 statement modifier:

# Good
bank_account.withdraw_funds! unless bankrupt?

# Bad
unless bankrupt?
   bank_account.withdraw_funds!
end

So, if you switch to this, it'll read easier.

if movie
  movie_library.catalog(movie)
  puts "Found: #{movie.title}. Score: #{movie.score}"
end

Is return 0 if first == last the best way to go about checking equaity in terms of working with the same object?

That works... though you could achieve the same if you did

return 0 if catalog.count <= 1

@jwo I renamed the #catalog to #add and moved the instance variable setting.

  def add(movie)
    movies << movie
    catalog = sort_by_year(movies)
    calculate_slope(catalog)
  end

Set instances like so:

def calculate_slope(catalog)
    first = catalog.first
    last = catalog.last
    return 0 if catalog.count <= 1
    @slope = (first.score - last.score).to_f / (last.year - first.year).to_f
  end

def sort_by_year(movies)
    @catalog = movies.sort { |movie,another_movie| movie.year <=> another_movie.year }
  end

Better?

As I'm reading the code, I am wondering if there is significance in the catalog of catalog = sort_by_year(movies)

If there is no significance, then I try to reduce down to the essence

  def add(movie)
    movies << movie
    calculate_slope sort_by_year(movies)
  end

Better?

MUCH. yay!

@jwo Awesome!
Hopefully I'll get better at sussing out these code smells.

@jwo calculate_slope sort_by_year(movies) will return the slope value? Is it okay if a method returns an object not inline with the method name?

Sure, check this example out: http://rubyfiddle.com/riddles/8f294

Of course, this calculate_scope sort_by_year(movies) is the same thing as

calculate_scope(sort_by_year(movies))

Just more parens

@jwo Hmmmmm. Your code looks great!
Seems I was caught up on the return values...