Skip to content

Instantly share code, notes, and snippets.

Show Gist options
  • Save philsof/f5ec77f42976f70801f5 to your computer and use it in GitHub Desktop.
Save philsof/f5ec77f42976f70801f5 to your computer and use it in GitHub Desktop.
Code review pair-dthelouie,plonsker parsing-data-1-csv-in-csv-out-challenge

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment