-
Notifications
You must be signed in to change notification settings - Fork 42
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
Pipes - Kee - API Muncher #28
base: master
Are you sure you want to change the base?
Conversation
API MuncherWhat We're Looking For
|
<!DOCTYPE html> | ||
<html lang="en"> | ||
<head> | ||
<meta charset="utf-8" /> |
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.
You shouldn't need to include all these <head>
tags outside application.html.erb
.
@query = params[:query] | ||
@results = EdamamApiWrapper.search_recipes(@query) | ||
if @results['hits'] != [] | ||
@results = @results['hits'] |
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.
This work of parsing the response should probably be done inside the API wrapper. One of the big advantages of structuring the code that way is that it isolates your program's dependency on the API, but here that dependency is leaking out.
Lib code is similar to model code in that you should push as much functionality there as possible.
|
||
response = HTTParty.get(url) | ||
|
||
return response.parsed_response |
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.
This method should be examining the response, checking for errors and returning a collection of Recipe
s, rather than the raw response.
|
||
if response.parsed_response == [] | ||
raise ApiError.new("Recipe does not exist") | ||
end |
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.
I like this method a lot more, but there are still some error cases you're missing. The big one in my mind is when the API keys are not set up correctly, for example if a new teammate were to download your project and miss that step of setup.
response = EdamamApiWrapper.search_recipes(query) | ||
response.must_be_kind_of Hash | ||
response.length.must_be :>, 0 | ||
end |
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.
This is a good success case, but I would be interested to see what happens when you search for something that returns no results.
proc { | ||
uri = "whatacrazyurihehehehehehe" | ||
EdamamApiWrapper.show_recipe(uri) | ||
}.must_raise EdamamApiWrapper::ApiError |
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.
Good failure case.
API Muncher
Congratulations! You're submitting your assignment!
Comprehension Questions