-
Notifications
You must be signed in to change notification settings - Fork 27
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
Please review my project! Thanks! #17
base: master
Are you sure you want to change the base?
Conversation
@director = @omdb_data["Director"] | ||
@actors = @omdb_data["Actors"] | ||
@link = "http://www.imdb.com/title/#{params["imdbID"]}" | ||
erb :show | ||
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 reviewed your app and everything looks good. The only thing that I would mention is the creation and use of all these instance variables that need to get passed to the views. You could just pass the one @omdb_data object to the show.erb view page and dereference the specific items there. In show.erb you could write
Plot: <%=@omdb_data["Plot"] %>
. As you have seen in Rails, we pass the "@todos" collection of todo items or "@todo" the one item to the view and then pluck out the details of the object in the views.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.
Ok, thanks for the feedback! I will definitely watch out for this in the
future. For some reason, I thought it was simpler to set them as variables
so that the "accessing" of the hash happened in the models, instead of in
erb in the view. But you're telling me that they are the effectively the
same, time-wise?
On Thu, Apr 24, 2014 at 10:21 AM, Spencer Eldred
[email protected]:
In movies.rb:
+get '/poster/:imdbID' do
- search = params[:imbdID]
- results = Typhoeus.get("http://www.omdbapi.com/?i=#{params[:imdbID]}")
- @omdb_data = JSON.parse(results.body)
- @poster = @omdb_data["Poster"]
- @plot = @omdb_data["Plot"]
- @released = @omdb_data["Released Year"]
- @Rated = @omdb_data["Rated"]
- @runtime = @omdb_data["Runtime"]
- @Genre = @omdb_data["Genre"]
- @director = @omdb_data["Director"]
- @actors = @omdb_data["Actors"]
- @link = "http://www.imdb.com/title/#{params["imdbID"]}"
- erb :show
endI reviewed your app and everything looks good. The only thing that I would
mention is the creation and use of all these instance variables that need
to get passed to the views. You could just pass the one @omdb_data object
to the show.erb view page and dereference the specific items there. In
show.erb you could write
- Plot: * <%=@omdb_data["Plot"] %> . As you have seen in Rails, we pass
the "@todos" collection of todo items or "@todo" the one item to the view
and then pluck out the details of the object in the views.Reply to this email directly or view it on GitHubhttps://github.com//pull/17/files#r11959991
.
Ms. Taylor Parker Chiu
[email protected]
650-245-3340
No description provided.