Emilce -Muncher (Consuming APIs)-Octos #35
Conversation
API MuncherWhat We're Looking For
This is a good start, but I think there's a way to go still. You were able to get the core API functionality working, which is good! However, there are some big holes still around error handling, testing and presentation. Remembering how to write "regular" Ruby code is part of the challenge of this assignment, but it seems like you're still struggling some of the Ruby fundamentals. Many of the problems I'm seeing are things that will hamper the rest of your development process. For example, having a good story around error handling makes everything else easier, because when something goes wrong you get a clear reason why instead of having to hunt for it. Fortunately, error handling and testing are big components of the VideoStore API project this week - make sure to spend some time focusing on this. These will continue to be important as we move into JavaScript next week, and having them down will make everything else you do a little easier. I know you've been putting in a ton of effort recently, and I want to acknowledge that. I do see improvements happening and progress being made. Keep up your focus and dedication, and things will continue falling into place. |
| root 'recipes#search' | ||
|
|
||
| get '/recipes', to: 'recipes#index', as: 'recipes' | ||
| get '/recipe', to: 'recipes#show', as: 'recipe' |
There was a problem hiding this comment.
Could you use resources for this?
| <meta name="viewport" content="width=device-width, initial-scale=1.0" /> | ||
|
|
||
| <title><%= content_for?(:title) ? yield(:title) : "Untitled" %></title> | ||
|
|
There was a problem hiding this comment.
This results in the page name being "Untitled"
| <body> | ||
|
|
||
| <%= yield %> | ||
|
|
There was a problem hiding this comment.
You should have something here to help give your site a unified look and feel - maybe a <header> with the search bar, a <footer> with the Edamam logo, and <main> tag wrapped around the yield.
| def self.raise_on_error(response) | ||
| if response["OK"] | ||
| raise EdamamError.new(response["error"]) | ||
| end |
There was a problem hiding this comment.
I like the idea of doing error handling in a method, but this doesn't match the way that Edamam reports errors. The OK and errors keys are what Slack did, but that's not going to be universal.
| @to = params[:to].nil? ? @from + MAX_PER_PAGE : params[:to].to_i | ||
|
|
||
| @recipes = RecipeApiWrapper.list_recipes(@query, @from, @to) | ||
|
|
There was a problem hiding this comment.
If you're expecting your lib file to raise an error, you should wrap this call in a begin/rescue.
| describe "list_recipes" do | ||
| it "lists recipes based on search" do | ||
| VCR.use_cassette("recipes") do | ||
| recipes = RecipeApiWrapper.list_recipes("ice cream") |
There was a problem hiding this comment.
What if a search yields no results?
| it "returns the details for a specific recipe" do | ||
| VCR.use_cassette("recipes") do | ||
| search = "ice cream" | ||
| response = RecipeApiWrapper.get_details("http://www.edamam.com/ontologies/edamam.owl#recipe_7bf4a371c6884d809682a72808da7dc2") |
There was a problem hiding this comment.
What if you try to get details with a bogus URI?
|
|
||
| describe RecipesControllerController do | ||
| # it "must be a real test" do | ||
| # flunk "Need real tests" |
There was a problem hiding this comment.
You need some controller tests! Here is my list of what I'm looking for:
Index:
- Valid search term
- Search term that produces no results (e.g.
adfohdaglhjalj) - No search term
Show:
- Valid URI
- Bogus URI
- No URI
Root
- Does it work at all (only one test)
API Muncher
Congratulations! You're submitting your assignment!
Comprehension Questions