Conversation
droberts-sea
left a comment
droberts-sea
left a comment
There was a problem hiding this comment.
We talked about this in our 1-on-1 today. Full review to come later in the week, once you've gotten a chance to update this.
| def self.find_recipe(params) | ||
| @recipe_list.each do |recipe| | ||
| if recipe.name == params | ||
| return recipe |
There was a problem hiding this comment.
This only works if the recipe being shown is in the most recent search results. If you open up two browser tabs and search for different things, you can only click on results from the last search.
The right thing to do here would be to send another request to Edamam, to get the details for that recipe. To do so you probably need something more distinct than just the recipe name - the URI might work nicely.
In general you should avoid using instance variables in class methods - these are basically global variables, and should only be used in very specific circumstances.
|
|
||
| root 'recipes#search' | ||
| get 'index', to: 'recipes#index', as: 'recipes_index' | ||
| get ':recipe/show', to: 'recipes#show', as: 'recipes_show' |
There was a problem hiding this comment.
These paths aren't quit RESTful. Why not use resources [:index, :show] here?
| <% end %> | ||
| <% end %> | ||
| </ul> | ||
| </class> |
API MuncherWhat We're Looking For
|
| <% @recipes.each do |recipe| %> | ||
| <li class="recipe_card"> | ||
| <div class="name"><h2><%= recipe.name %></h2></div> | ||
| <img src=<%= recipe.image %> class="thumbnail_photo"> |
There was a problem hiding this comment.
Suggestion: Use image_tag and make the image a hyperlink. It's human nature to want to click on the link.
3 participants
API Muncher
Congratulations! You're submitting your assignment!
Comprehension Questions
lib? How would your project change if you needed to interact with more than one API (aka more than just the Edamam API)?