Pottery (fire + earth): Ren and Anya

[anyatokar]

Assignment Submission: Rideshare Rails

Congratulations! You're submitting your assignment. Please reflect on the assignment with these questions. These questions should be answered by all team members together, not by a single teammate.

Reflection

Prompt	Response
Describe the types of entity relationships you set up in your project and why you set up the relationships that way	Driver and passenger have multiple trips to satisfy the needs of the client cards. Trips to passenger/driver relationship is one-to-many, trip belongs to driver/passenger.
Describe the role of model validations in your application	To make sure only expected parameters can be passed. Unexpected parameters can introduce bugs to the program or malware.
How did your team break up the work to be done?	We worked on the fundamental setup together. Then we split up methods, testing, html/css between us. We came back together when we got stuck on things.
What features did you choose to prioritize in your project, and what features, if any, did you have to set aside to meet the deadline?	We set aside refactoring existing validation tests to fit our methods. We prioritized connecting the three models, and making sure basic app functioning was met (CRUD).
What was one thing that your team collectively gained more clarity on after completing this assignment?	We gained more clarity on nested routes and writing methods to fit connected models. There are a lot of different ways to approach meeting client card needs.
What are two discussion points that you and your team discussed when giving/receiving feedback from each other that you would be willing to share?	We gained understanding about the importance of knowing the methods when writing the tests and vice versa (not in a vacuum).
Optional: What is the URL of your deployed Heroku app?	http://pottery-rideshare.herokuapp.com/

[kaidamasaki]

Excellent job! Everything looked pretty solid except for the couple of things I called out. The main thing is that you seem to have stored all of your data in the database as strings.

Aside from that though your code was clean and well organized!

Rideshare Rails

Major Learning Goals/Code Review

Criteria	yes/no, and optionally any details/lines of code to reference
Practices collaborating with git, and all team members contribute git commits and best git practices	✔️
Demonstrates understanding of relationships by giving accurate answers to the reflection questions and appropriate code in the models	✔️
Logic to calculate a driver's average rating and total earnings is located in the model, and it has unit tests	✔️
There are reasonable tests to test the validation requirements for all models	Validations are not fully tested.
There are reasonable tests to test the relationship requirements for all models	✔️
There are reasonable tests to test the controller actions for all controllers	✔️
The app has an attractive and usable user interface	✔️
The app uses/is compatible with database seeds	✔️
Router code is clean: uses resources and RESTful routes	✔️

Functional Requirements

Functional Requirement	yes/no
On the passenger's details page, I want to be able to see total charged, list of trips, a link to edit, and delete	✔️
When adding a new passenger, I want to see errors and validations that show that a passenger must be provided a name and a phone number, so that I cannot make a passenger without name or phone number	✔️
On the passenger's details page, I can create a new trip for this passenger, with an assigned driver and no rating	✔️
On the driver's details page, I want to be able to see total earnings, average rating, list of trips, a link to edit, and delete	✔️
When adding a new driver, I want to see errors and validations that show that a driver must be provided a name and VIN, so that I cannot make a driver without name or VIN	✔️
On the trip's detail page, I want to be able to view details, assign a rating, navigate to the trip's passenger, driver, a link to edit, and delete	Ratings can't be assigned from the details page.

Overall Feedback

Overall Feedback	Criteria	yes/no
Green (Meets/Exceeds Standards)	8+ in Code Review && 5+ in Functional Requirements	✔️
Yellow (Approaches Standards)	5+ in Code Review && 4+ in Functional Requirements, or the instructor judges that this project needs special attention
Red (Not at Standard)	0-4 in Code Review or 0-3 in Functional Reqs, or assignment is breaking/doesn’t run with less than 5 minutes of debugging, or the instructor judges that this project needs special attention

Code Style Bonus Awards

Was the code particularly impressive in code style for any of these reasons (or more...?)

Quality	Yes?
Perfect Indentation	✅
Elegant/Clever	✅
Descriptive/Readable	✅
Concise	✅
Logical/Organized	✅

[kaidamasaki]

I like verifying that your assumptions are true. I'd probably put the expect into the "arrange" section like you did above though.

[kaidamasaki]

You should use a more meaningful name than "red" here.

If the intent is that this is an error you should name the key "error".

[kaidamasaki]

It's confusing that sometimes you use "red" for success.

[kaidamasaki]

Your class names shouldn't talk about what style they produce.

For example, if you decided to change your success indicator to be an icon instead of a color this would no longer make sense as a class name.

Something like success-message would be preferable:

Suggested change

	section.flash div.green-message {
	section.flash div.success-message {

[kaidamasaki]

Since you only have two legal values "available" should have been a boolean, not a string:

Suggested change

	t.string :available
	t.boolean :available

[kaidamasaki]

You can clean this up with enumerable methods (compact is actually an array method):

Suggested change

	def average_rating
	nil_counter = 0
	rating_sum = 0
	self.trips.each do |trip|
	if trip.rating.nil?
	nil_counter += 1
	else
	rating_sum += trip.rating.to_f
	end
	end
	num_ratings = (self.trips.length) - nil_counter
	average = (rating_sum/num_ratings.to_f).round(1)
	return average
	end
	def average_rating
	ratings = self.trips.compact.map { |t| t.rating.to_f }
	return (ratings.sum / ratings.length).round(1)
	end

[kaidamasaki]

You don't need to return 0 here; inject returns 0 if the list is empty.

[kaidamasaki]

This can be simplified with a sum:

Suggested change

	def total_spent
	total = 0
	self.trips.each do |trip|
	total += trip.cost.to_i
	end
	return total
	end
	def total_spent
	return self.trips.sum do |trip|
	trip.cost.to_i
	end
	end

[kaidamasaki]

These should have been numbers:

Suggested change

	t.string :rating
	t.string :cost
	t.integer :rating
	t.float :cost

[kaidamasaki]

This is the kind of code that would have been simpler if you had stored this in the database as a boolean.