Skip to content

Instantly share code, notes, and snippets.

@philsof
philsof / code-review-pair-kerryimai,plonsker-orange-tree-2-groves-challenge.md
Last active January 26, 2016 06:17
code-review-pair-kerryimai,plonsker-orange-tree-2-groves-challenge

Good stuff getting to this challenge! Here are my notes on your code:

  • It looks like you have methods on your specific tree classes that are similar. For example, OrangeTree.has_oranges? and PearTree.has_pears?. This is a code smell. You could DRY out your code by putting a has_fruit? method on the FruitTree class. This way, every child class of FruitTree will have a has_fruit? method. Tada! Can you find any other ways to DRY out your code? Hint: look for similarly-worded methods that appear in all of your tree classes.

  • Each of your tree classes has a few lines of code in its age! method that needs to be rethought. Here are those lines, from the AppleTree class:

    if age > 3 && age < 17
      new_number = rand(10..
@philsof
philsof / code-review-mirascarvalone-ruby-racer-1-outrageous-fortune-challenge.md
Last active January 26, 2016 06:17
code-review-mirascarvalone-ruby-racer-1-outrageous-fortune-challenge

Good work on this! Your code works; it does exactly what the challenge asks for. And your methods all have a single responsibility and are easy to read. Some notes on details:

@philsof
philsof / code-review-pair-espezua,superboyblue-active-record-drill-model-a-student-challenge.md
Last active January 26, 2016 06:18
code-review-pair-espezua,superboyblue-active-record-drill-model-a-student-challenge

This is good stuff! Your code passes all of the tests! Here are some notes:

  • Nice handling of full names with 2+ words! Your code can handle any crazy long name that comes its way. Sweet!
  • Nice custom validation method age_more_than_five
  • This may be picky, but FYI: Your regexp for your phone number validation has spaces in it. The best way to reference the space character in a Ruby regexp is \s (although according to this table, the space character will work, as it does in your code). Just keep this in mind for best practices.
  • In your [full_name method]
@philsof
philsof / code-review-pair-dandersen2,mirascarvalone-poll-database-design-challenge.md
Last active January 26, 2016 06:18
code-review-pair-dandersen2,mirascarvalone-poll-database-design-challenge

Your schema is solid, good work! Your Users_and_Polls table is the key to your schema design. You realized the need for a table that contains the unique combination of one user, one poll, and one response. And you did so without compromising the poll creating association (one user can create many polls, but each poll can only be created by one user) and the response creating association (one poll can have many responses, but each response can only have one poll) and the voting association (one user can vote on many polls, and one poll can be voted on by many users). Do you realize what this means? It means you are really getting this database design stuff. CONGRATS!

Here are some tips to sharpen everything:

  • The default names of your tables need to conform to naming convention. Table names should be plural, lowercase, with _ between
@philsof
philsof / code-review-for-pair-edwardgemson-university-course-database-design-challenge.md
Last active January 26, 2016 06:18
code-review-for-pair-edwardgemson-university-course-database-design-challenge

Good effort! Ok there are a bunch of issues with your schema. It looks like you got confused when implementing the sections. Take a look at your relationships, and remember: the "one" side of a relationship is the side that has the id primary key, and the "many" side is the side that has the foreign key (which is always [name-of-other-table]_id)

So right now, these are your assocations: ###Good associations:###

  • a student can have many classes, and a class can have many students [GOOD; this is the red line and black line attached to the students_classes table]
  • one class can have many sections, and a section can have only one class [GOOD; this is the black line attached to the sections table]
  • a teacher can have many sections, and a section can have only one teacher [GOOD; this is the dark blue line]

###Half good, half not good

@philsof
philsof / code-review-solokerry-roman-numerals-challenge.md
Last active January 26, 2016 06:18
code-review-solokerry-roman-numerals-challenge

Good work on this! Your code works, it successfully converts Arabic numbers into Roman numerals. Success!

Some notes:

  • This challenge is testing your ability to utilize hashes and the / and % operators, which you do well.

  • How could you better utilize these operators in your code? Here is a really short solution that further taps into the capabilities of / and %:

    def to_roman(num)
      roman_num = {1000 => "M", 900 => "CM", 500 => "D",400 => "CD",100 => "C",90 => "XC" ,50 => "L",40 => "XL", 10 => "X",9=>"IX", 5 => "V",4=>"IV",1 => "I" }

roman_num_string = ""

@philsof
philsof / code-review-pair-jesskrich,san115-scraping-hn-1-building-objects-challenge.md
Last active January 26, 2016 06:18
Code review for pair-jesskrich,san115 branch on scraping-hn-1-building-objects-challenge

Good work!

##The good stuff:##

  • Very nice that you created separate to_s methods for your post class and comment class, and that your post class to_s utilizes the comment class to_s. Very sharp separation of responsibilities!
  • Very wise and clean to separate the parsing of the data and creating of the objects from the rest of your code. Also I like the names of your modules because they clearly convey what each module is doing: PostFactory and CommentFactory. These modules show you are grasping what it means to separate responsibilities.
  • Good use of map_with_index here

##Stuff to work on:##

@philsof
philsof / code-review-pair-mirascarvalone,superboyblue-scraping-hn-1-building-objects-challenge.md
Last active January 26, 2016 06:18
Code review for pair-mirascarvalone,superboyblue branch on scraping-hn-1-building-objects-challenge

Good effort! Here are my notes on your code:

  • I can't find any runner code in your branch. (There is a runner.rb file, but when I run it, nothing happens.) When I add my own runner code to your runner file (by adding the line puts post to the end of your runner file), and then run the file, I get this output:
A/B testing mistakes | Hacker News (Hacker News ID: 5003980)
    URL: http://visualwebsiteoptimizer.com/split-testing-blog/seven-ab-testing-mistakes-to-stop-in-2013/
    Author: ankneo
    Points: 53

    Comments:
@philsof
philsof / code-review-pair-espezua,mattopps-scraping-hn-1-building-objects-challenge.md
Last active January 26, 2016 06:18
Code Review for pair-espezua,mattopps branch on scraping-hn-1-building-objects-challenge

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](https://github.com/nyc-chorus-frogs-2016/scraping-hn-1-building-objects-challenge/blob/pair-espezua%2Cmatto
@philsof
philsof / code-review-plonsker_and_kb-for-scraping-hn-1-building-objects-challenge.md
Last active January 26, 2016 06:18
code-review-plonsker_and_kb-for-scraping-hn-1-building-objects-challenge

Good effort on this! Here are my notes:

  • Your runner code (which appears to be in your parser.rb file) does not seem to be utilizing any of your post class code. It is not creating a post object or printing any post info.
  • There is code in your post initialize method that is most likely not doing what you think it is doing, right here. This is because of how your add_comment method is written (see next point).
  • In your post class you have a very interesting add_comment method, which is this:
def add_comment(comment)
  symbol_id = ("id" + comment.id.to_s).to_sym
  @comments[symbol_id] = comment
end