Code review pair-dthelouie,plonsker parsing-data-1-csv-in-csv-out-challenge

code-review-pair-dthelouie,plonsker-parsing-data-1-csv-in-csv-out-challenge.md

This does the job. Good work. Here are my notes on your code:

• It looks like you unnecessarily separated the parsing work into two methods, your get_list method and your parse_people_objects_from_file method. All of this logic could be contained within parse_people_objects_from_file; there is no need to separate some of the logic into a get_list method. This is how you could combine both methods:

  def parse_people_objects_from_file
    csv_row_objects = []
    CSV.foreach('people.csv', headers: true) do |line|
      csv_row_objects.push(line)
    end
    people_objects = []
    csv_row_objects.each do |csv_row|
      person_object = Person.new(csv_row['first_name'], csv_row['last_name'], csv_row['email'], csv_row['phone'], csv_row['created_at'])
      person_object.created_at =  DateTime.parse(csv_row['created_at'])
      people_objects << person_object
    end
    return people_objects
  end

• Be sure to delete any code you don't want in your final product, like this and any commented out code no longer being utilized.

• It would be best to set up your Person initialize method so that it is not dependent on argument order. More info on how to this, and why you should do this, here.

• Your runner code for saving new data to the CSV file works, good job. Just watch out: you are not parsing the created_at into a DateTime object, so when you try to parse any new data back in, you will get a type error, since the created_at may not be valid for parsing into a DateTime object.

• Also for the runner code: your CSV.open code should be in your PersonParser, not in your runner file; the actual work/logic should be taken care of in your classes and modules, not in your runner file. The runner file should call methods that live in your classes and modules, not actually house those methods.

Good work! Any questions let me know.

-Phil