Skip to content

Instantly share code, notes, and snippets.

@philsof
philsof / code-review-for-pair-dandersen2,edwardgemson-from-scraping-hn-1-building-objects-challenge.md
Last active January 26, 2016 06:18
Code review for pair-dandersen2,edwardgemson from scraping-hn-1-building-objects-challenge

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
@philsof
philsof / code-review-for-pair-kerryimai,yilu1021-scraping-hn-1-building-objects-challenge.md
Last active January 26, 2016 06:18
Code review for pair-kerryimai,yilu1021 branch: scraping-hn-1-building-objects-challenge

Your code works! Good job! Here are some notes:

  • Line 3 in your comment.rb file sets two attribute readers: attr_reader :comment_user_ID, :comment_text. These attribute names are redundant since they are attributes on your comment class. Removing the word 'comment' from the attribute names and simply naming them attr_reader :user_ID, :text would suffice.
  • Lines 12-14 in your post class can be refactored. Instead of this:
  def comments
    @collection_of_comments
  end

you could eliminate the need to write this method by naming your collection @comments and create an attr_reader for it: attr_reader :comments

@philsof
philsof / pair-jesskrich-cookies-and-ovens-challenge.md
Last active January 26, 2016 06:18
Code review for pair-jesskrich on cookies-and-ovens-challenge

Good effort on this challenge! Here are some comments from me:

###Release 0: Move cookies into and out of the oven###

  • I can move cookies into your oven, but not out of it. Take a look at your ``remove_cookies``` method:

    def remove_cookies
      @contents.each do |cookie|
        if cookie.done == true
          @cooling_rack << cookie
@philsof
philsof / miniQuery-challenge-sasha-jeff.md
Last active January 26, 2016 06:18
Code review for pair-Sasha,Jeff for miniQuery-challenge

Good work on releases 0 and 1!

A few notes:

###It Works!###

  • Your code does everything that releases 0 and 1 ask for. Success!

###Formatting###

  • Your JavaScript indentation is perfect. That makes your code very easy to read, which is greatly appreciated. Readability is vital!
@philsof
philsof / orange-trees-1-pair-san115,superboyblue.md
Last active January 26, 2016 06:18
Code review of Orange Tree 1 challenge for pair-san115,superboyblue branch

Good work! Here are my comments on your code:

Orange Tree class:

age! method: Right now, all of the oranges you create have a diameter of 3. If you want to keep it like this, you could make your code simpler by hard coding the diameter into your Orange class initialize method. Or (much better!), perhaps you could change this method so that each orange is created with a random diameter (like in real life). It would be cool if you did that! (And heads up: the runner.rb file assumes your oranges will have varying diameters. See my note on the runner.rb file below.)

Also heads up: Looks like line 25 will add oranges to a tree even if the tree is already dead (!). Uh oh! How could you rewrite your code to avoid this from happening?

Also you should utilize your attr_writer :dead method here. On line 19, instead of @dead = true you could utilize your attr method by writing this as self.dead=(true) (This samem concept also applies to all of your references to grown_oranges and `