Skip to content
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

API Muncher - Katrina - Edges #27

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open

Conversation

kaganon
Copy link

@kaganon kaganon commented Nov 5, 2018

API Muncher

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
How did you go about exploring the Edamam API, and how did you try querying the API? I mostly used Postman and the rails console for querying the API request
What is an API Wrapper? Why is it in lib? How would your project change if you needed to interact with more than one API (aka more than just the Edamam API)? The API Wrapper is a Ruby class that is used to get data from API requests. It's in the lib file because we have to configure the custom library ourselves. If there were more than one API, we'd have to create a wrapper for each API.
Describe your API wrapper, the methods you created in it, and why you decided to create those methods/how they were helpful The methods in my API wrapper include finding the list of recipes and finding one individual recipe. It was helpful to have the methods separated because the data returned from each request was a little bit different from each other.
What was an edge case or failure case test you wrote for your API Wrapper? What was a nominal case? A failure test for my API Wrapper was testing for an invalid recipe uri. It tested that it returned an empty array if the uri was invalid. A nominal case was that it returned the recipe if it was a valid uri.
How does VCR aid in testing an API? It helps because it records a response and we don't have to keep making API calls for every test. It's cost efficient and it works without depending on external sources.
What is the Heroku URL of your deployed application? https://muncher-api.herokuapp.com/

@droberts-sea
Copy link

API Muncher

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene yes
Comprehension questions yes
General
Rails fundamentals (RESTful routing, use of named paths) yes - good use of resources
Semantic HTML mostly - see inline
Errors are reported to the user yes - good work
API Wrapper to handle the API requests yes
Controller testing yes
Lib testing yes
Search Functionality yes
List Functionality yes
Show individual item functionality yes
Styling
List view shows 10 items at a time and/or has pagination yes
Attractive and usable UI yes
API Features
The App attributes Edamam yes
The VCR cassettes do not contain the API key yes
External Resources
Link to deployed app on Heroku yes
Overall Great work overall! This app is attractive, functional and well-tested, and it's clear to me that the learning goals for this assignment were met. As with any codebase there are things that could be cleaned up, which I've tried to mark inline, but in general I am quite pleased with this submission. Keep up the hard work!

if data.first
return [details_recipe(data.first)]
else
return []

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why put the result in an array here - the method name find_recipe implies a single result. Instead I would return the single Recipe instance by itself, or nil if not found (similar to the behavior of find_by).

def self.details_recipe(data)
return Recipe.new(
data["label"],
URI(data["uri"]),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This and create_recipe are doing very similar work - could you consolidate them?

private

def self.create_recipe(api_params)
return Recipe.new(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that you've broken this out as a separate method and made it private - good organization.

<section class='header-container'>

<div class="header-text">
<a href="<%= root_path %>"><h1>MUNCHER</h1></a>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not make this a <header>?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants