Code review for pair-dandersen2,edwardgemson from scraping-hn-1-building-objects-challenge

code-review-for-pair-dandersen2,edwardgemson-from-scraping-hn-1-building-objects-challenge.md

This is really good stuff. I am going to get into nitty gritty to give you some items you could tweak:

• Your Parsable module is statically referencing two instance variables that are being created outside this module: @nokogiri_document and @file. That is a no-no. You want your module to be written in such a way that it needs to know nothing about the rest of your code. Thus, a better way would be to pass the nokogiri document and the file to the module as arguments. As in other words, your Parsable module should be written in such a way that, if the names of these two instance variables were changed, there would be no need to change any of the code in your Parsable module.
• The map_with_index on line 5 in your Parsable module is sweet. Nice work iterating over the DOM like this and delivering an array of comment objects to your post initialize method.
• Heads up: Your to_s method on your post class only prints the state of the comment objects encapusulated in your post object; it doesn't print the rest of the state of your post object. You should add that to the to_s method so it prints the title, url, etc. of your post object.
• It is really cool that you utilized colors!
• Also the big sign that your code is tight is that your runner.rb file is so short. Nice work.

##Challenge for you:##

Refactor your code in such a way that these two conditions are met:

1. This runner code would work:

post = Post.new({file:'html-samples/hacker-news-post.html'})
puts post

2. The only instance variables referenced in your code are from the same class they are being referenced in. For example, in your post initialize method, this line would be cool because it is referencing an instance variable that is also created by your post class (the @html_node instance variable):

@body_node = @html_node.children.last

but this line would need to be refactored because it is currently referencing a variable created outside of your post class (the @nokogiri_document instance variable:

@html_node = @nokogiri_document.children.last

You can do it!

Any questions let me know.

-Phil