-
Notifications
You must be signed in to change notification settings - Fork 25
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
Assignment #2 #8
base: master
Are you sure you want to change the base?
Conversation
@@ -9,6 +9,9 @@ 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) | |||
|
|||
return "Not Found" if struct.id.nil? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here you're returning a string that has a semantic meaning of "nothing was found". So this method could either return:
- string with "Not Found"
- Movie object with information
I think either raising an exception, or a specific NilMovie would be more expressive of what happened.
Left you a suggestion on how you're handling the not-found... looks promising! Let me know if you have questions on the string vs object stuff. |
@jwo Jesse, I've updated the way I raise the exception. I think this looks better, right? |
@annavester I think it's better for sure. I would recommend that instead of catching the NotMethodError, we try to catch something a little more close to the domain. maybe something like this:
Now, the API will raise a MovieNotFoundError, and you can catch it in the movie_json workflow and handle it there. Maybe:
|
Here is my Panda Level, I am still working on the rest... Thanks!