Code Review for pair-espezua,mattopps branch on scraping-hn-1-building-objects-challenge

code-review-pair-espezua,mattopps-scraping-hn-1-building-objects-challenge.md

Your code works! Good job!

Here are some notes from me:

• I like the attribute naming on your post object, and the cleanness of your add_comments method, and the utilization of a to_s method on your post object, and the use of an args hash for your initialize arguments, and the cleanness of your encapsulating your comment objects inside your post object. Well done!
• One of the attribute names on your comment class is a little redundant: comment.user_comment. Since we already know this is an attribute of a comment, we can name it something like comment.text or comment.content. (This attribute is the text or content of the comment. To say that it is the user_comment of the comment is too vague and redundant.)
• A lot of your runner code is not really running anything but rather parsing data and then setting up that data for your object new methods. It would be better if this parsing were occuring in a class or, even better, in a module. Then the runner code would simply call that parser module. (You want your runner code to do only one thing: run code. The actual work should be done inside your classes and modules.)

##Some tips on using to_s##

Nice use of to_s on your post object! Note though: If you call puts on an object (for example: puts post), Ruby will automatically puts the return value of that object's to_s method. (Thus, there is no need to have any puts calls inside your to_s method.) Because of this, you could refactor your post object's current to_s method into something like this:

def to_s
  array_of_comment_strings = @comments.map  do |comment|
    "#{comment.user_comment}\n\n-#{comment.user_id}\n\n"
  end

  "#{title} (Hacker News ID: #{item_id})\nURL: #{url}\nAuthor: #{author_username}\nPoints: #{points}\n\nComments:\n" + array_of_comment_strings.join("\n")
end

Or, even better, you could also write a to_s method for your comment objects, and call that to_s method in your post object's to_s method. So you could write a to_s for your comment objects like this:

def to_s
  "#{user_comment}\n\n-#{user_id}\n\n"
end

And then call that to_s from within your post to_s, so that your post to_s would now look like this:

def to_s
  array_of_comment_strings = @comments.map  do |comment|
    comment.to_s
  end

  "#{title} (Hacker News ID: #{item_id})\nURL: #{url}\nAuthor: #{author_username}\nPoints: #{points}\n\nComments:\n" + array_of_comment_strings.join("\n")
end

This way, each of your classes has its own to_s method, which is much cleaner than only one class having a to_s method.

Also note: Because puts post will puts the return value of the post to_s method, you can change the last line of your runner code to simply:

puts post

I hope this helps. Any questions let me know. -Phil