new way of handling film data

[JRJurman]

films now only require a url, and then can take an optional map of details that letteropend will use to make assumptions, before actually pulling from letterboxd.com

[JRJurman]

Many of the examples still need to be updated, which I'll be committing next, for now, take a look at the FIlm.rb, and the getFilmDetails.rb example

[JRJurman]

This PR is now ready for review

[nickserv]

How is back compatibility affected? Is this because the method signatures for Film changed?

[JRJurman]

Yes, the previous implementation

Letteropend::Film.new(title, url, callbacks={})

is now

Letteropend::Film.new(url, details={}, callbacks={})

Any conversions that need to be made can be done in the following fashion

deprecated = Letteropend::Film.new("Robocop", "/film/robocop/")
newVersion = Letteropend::Film.new("/film/robocop/", {:title=>"Robocop"})

[nickserv]

It's often a convention in Ruby to always have all optional arguments in one hash at the end of the parameters. This way, you don't have to type {} when using the hash params. How about merging details and callbacks? There aren't many callbacks anyway.

[JRJurman]

Honestly, the more I think about it, the more I'm less of a fan of the callbacks hash... Is there a more "ruby" way of doing this?

Would it make sense to have the users define methods on the class instead?

# example.rb
class Letteropend::Film
  def on_updated_model
    puts "The film was updated!"
  end
end

f = Letteropend::Film.new("/film/good-burger/")
title = f.title         # => prints to console "The film was updated"
puts title             # => prints to console "Good Burger"

[JRJurman]

@kristenmills what do you think?

[kristenmills]

Letting the user open up the class is not the way to go.

[kristenmills]

Neither is the callback hash, that's weird. Do something with the method block to define callbacks. Don't worry my next post will be a suggestion.

PS. No rocket syntax.

Good

hash = { key: value }

Bad

hash = { :key => value }

[JRJurman]

I have no idea what that would look like... example? Thanks

[kristenmills]

It's coming :smile:

[kristenmills]

film = film 'good-burger', detail_1: 'value', detail_2: 'value' do
  on :model_updated do
    puts "Model was updated"
  end
end

I took out the '/film/' because that should be implied. You can define a set of callback types and they can be applied accordingly

[kristenmills]

If you don't want to polute the global namespace with a film method, that can be replaced with Letteropened::Film.new

[kristenmills]

Maybe if you want defaults we have this option as well

Letteropened::Film.config do
  on :model_updated do
    puts "Model was updated"
  end
end

[JRJurman]

I like this... and yet... have no idea how to do this... what is the "callback" doing in these examples?

[kristenmills]

Basically that's how you would define your callbacks. They would be specific to an instance of the class. I can point you to some resources on how to make a DSL and show you some that I made. It's a good skill to learn.

Note: I changed the syntax in my previous comments

[JRJurman]

@kristenmills I made a commit that functions how I believed you described above, if you (and/or other people) could validate that I'm not doing anything super janky, that would be most bodacious!

[kristenmills]

Looks solid to me. Anyone else have any opinions on it? @nicolasmccurdy specifically.

[JRJurman]

I updated the other examples, and removed the get_total_runtime from the list object, seeing as that functionality can really be implemented by the user, and doesn't need to be built-in

[nickserv]

:+1: Looks good the me! The config syntax is pretty cool.