Feedback for Tiffany

feedback.md

README

Give your README some love. It's the first thing someone sees when they open your app. It doesn't need anything fancy, but at least give some basic setup instructions to get the app up and running (ruby version, bundling, rails server, how to run specs, etc). Basically just answer the questions that are in there as boilerplate. There may be developers that aren't familiar with rails reviewing your app and they'll appreciate it.

Gemfile

• Remove anything you don't need
  • turbolinks
  • jbuilder
  • sdoc
  • sqlite3
• A lot of stuff in the development & test group are only needed in test
• Use the latest stable version of rails
• The instructions say "Please don't minify any of the JavaScript". Should you rip out uglifier from here and production.rb?

StoriesController

• #stories doesn't need tom be memoized

GetStories

• Why do you have to pass the url into GetStories? It's only ever called with the one url, so why not just push it down into GetStories?
• #parsed_response can be private
• When you're mapping over articles, it would make more sense if each iteration you were working with an article, not a story
• Use URI.join to build a the image url

Story

I think this class makes more sense in app/models. A story seems like part of your domain to me.

ArticleFilter

• If you're following the rules we use for services (which NYT doesn't know about, so it's not as big a deal), This should probably have a verb name like FilterArticles
• Can you think of a name besides response for what gets passed in? This class shouldn't care that the object it's being passed came from a network call.
• There's a lot of nested iterators in here. Maybe you can use #reduce or #each_with_object in #contents_with_assets to clear it up. Or maybe another object to do some of the work.

StoriesHelper

• Use this regex to determine if a letter is capitalized /[[:upper:]]/

Translating to Martian

You have martian translation logic in your JS, which the instructions say shoudl be done on the server side. If you want to actually do this on the server side, you might want to consider exposing an api endpoint that takes some agreed upon data structure from the client, converts all text to martian, and returns the same data structure.