Skip to content

Queues - Cynthia Cobb - Ride Share - #34

Open
cynthiacd wants to merge 54 commits into
Ada-C7:masterfrom
cynthiacd:master
Open

Queues - Cynthia Cobb - Ride Share#34
cynthiacd wants to merge 54 commits into
Ada-C7:masterfrom
cynthiacd:master

Conversation

@cynthiacd

Copy link
Copy Markdown

Ride Share

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
Describe a design decision you had to make when working on this project. What options were you considering? What helped you make your final decision? I decided to read the csv file in a separate class so I could inject this data in - I did this so I could more easily test mock data. I also found all 3 of my classes had similar overall design, and if I had time would create a class that rider, driver, trip could inherit from
Describe a concept that you gained more clarity on as you worked on this assignment. I felt I learned a lot about writing specs - I worked really hard to make sure if any irregular data got passed in, an argument error would be raised. While going through this process I learned more about writing specs. I experimented with using sample in specs, and having specs check for a specific error message
Describe a nominal test that you wrote for this assignment. I made sure each method has a nominal test - for example the calculate average method tests a sample of 25 drivers and makes sure their averages fall between the range of 1-5
Describe an edge case test that you wrote for this assignment. I had a tougher time with edge cases the data was so similar so it was hard to find examples of where the method should not raise errors but might behave differently - I did make sure to write specs for all methods that should return nil when applicable
How do you feel you did in writing pseudocode first, then writing the tests and then the code? I liked writing out a diagram of the classes before hand and writing what I wanted of each method

cynthiacd added 30 commits March 6, 2017 14:52
…plemented specs for Trip class method find_by_rider
… is an array (not going to read csv file from the Trip class
…bad data to the all method (which initializes all the instances at this point in the program)
…of data to all and find instead of the csv file
…the dependency between classes - going to now fully embrace the dependencies
…nt have any corresponding trip or driver instances
cynthiacd added 24 commits March 9, 2017 16:41
@droberts-sea

Copy link
Copy Markdown

Ride Share

What We're Looking For

Feature Feedback
Baseline
Used Git Regularly yes - good work!
Answer comprehension questions yes
Driver
Uses the all method in the find method yes
Has appropriate edge-case tests for each method in the class yes
Created a method that uses a method from the Trip object to retrieve the list of trips yes
Created a method that uses the internal trips list to calculate the average rating yes
Rider
Uses the all method in the find method yes
Has appropriate edge-case tests for each method in the class yes
Created a method that uses a method from the Trip object to retrieve the list of trips yes
Created a method that uses the internal trips method to retrieve the associated drivers yes
Trip
Reads the CSV file in the all method yes
Has appropriate edge-case tests for each method in the class yes
Created a method that uses a method from the Driver to retrieve the associated driver instance yes
Created a method that uses a method from the Rider to retrieve the associated rider instance yes
Created a method to retrieve all trips by driver id yes
Created a method to retrieve all trips by rider id yes
Overall

Great job! Code is clear and readable, tests cover everything I was looking for, and git habits look solid. Keep up the good work!

Comment thread lib/driver.rb
name
end

def self.test_for_vin(vin)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work abstracting this logic out to its own method. This makes the code easier to read, and easier to update in the future (for example, if you needed to introduce more checks on what a VIN is).

Comment thread specs/driver_spec.rb

describe "Driver" do

describe "Driver#initialize" do

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good organization - the nested describes make this code much easier to read.

Comment thread specs/driver_spec.rb

@bad_data = {
bad_id: [['2', 'Emory Rosenbaum', '1B9WEX2R92R12900E'], ['ten', 'name', 'WBWSS52P9NEYLVDE9']],
bad_vin: [['10', 'name', 'WBWSS52P9NE']],

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good organization here. Very obvious why various combos are bad.

Side note: doesn't bad_vin also have a bad ID (since it's a string)? Does this invalidate your test?

Comment thread lib/file.rb
require 'csv'
#this class creates a FileData object which is an object that has csv_file and
# you can call read method(s) on this object
class FileData

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea of moving all this to its own class. You might be particularly interested in the last part of the FarMar live code video at https://adaacademy.hosted.panopto.com/Panopto/Pages/Viewer.aspx?id=2c336ca7-b2a8-4a78-941b-cd297639b17d

Comment thread specs/driver_spec.rb
driver_know_avg.calculate_average_rating.must_equal 2.73
end

it "returns nil if there are no trips for driver" do

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good test case! Edge cases like this, asking what happens when there isn't any data, can be easy to miss.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants