Skip to content

Pipes - Lauren and Angela - Ride Share Rails#19

Open
awilson2017 wants to merge 93 commits into
Ada-C8:masterfrom
awilson2017:master
Open

Pipes - Lauren and Angela - Ride Share Rails#19
awilson2017 wants to merge 93 commits into
Ada-C8:masterfrom
awilson2017:master

Conversation

@awilson2017

Copy link
Copy Markdown

Rideshare-Rails

Congratulations! You're submitting your assignment! These comprehension questions should be answered by both partners together, not by a single teammate.

Comprehension Questions

Question Answer
Describe the types of entity relationships you set up in your project and why you set up the relationships that way Trip belongs to driver and passenger, passenger has many trips and drivers has many trips.
Describe the role of model validations in your application They validate user input like rating (nil or integer), passenger and driver enter both names and numbers associated to them.
How did your team break up the work to be done? Please refer to our Trello board for how we divided the work into manageable tasks. As for as delegation, sometimes we worked together and sometimes apart.
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 prioritized the functionality of the app first and foremost. We did not touch CSS until everything ran smoothly and all requirements were met. We did basic CRUD the first day and then spent majority of the week on understanding the relationships between the models and creating a trip!
What was one thing that your team collectively gained more clarity on after completing this assignment? We learned more about models and the code that lives within them, Trello, and improved communication.
What is your Trello URL? https://trello.com/b/1JoqumnA/rideshare
What is the Heroku URL of your deployed application? ride-with-ada.herokuapp.com

awilson2017 and others added 30 commits October 2, 2017 15:11
… trip, driver of trip, and rating of trip. Also displayed total cost.
@droberts-sea

Copy link
Copy Markdown

Rideshare-Rails

What We're Looking For

Feature Feedback
Baseline
Appropriate Git Usage with no extraneous files checked in and both partners contributing yes
Answered comprehension questions yes
Uses named routes (like _path) yes
RESTful routes utilized yes - good use of nested routes
Rideshare Rails Specific Content
Table relationships yes
Validation rules for Models yes - it seems a little odd to me that you can't have two passengers or drivers with the same name
Business logic is in the models some - see inline comments
Database is seeded from the CSV files yes
Trello board is created and utilized in project management yes
Postgres database is used yes
Heroku instance is online yes
The app is styled to create an attractive user interface yes - great work! Love the theme!
Overall Good work overall. There is some room for improvement around organizing business logic in the model, but in general I'm quite happy with the project you've submitted.

<%= link_to 'Delete Passenger',
passenger_path(:passenger_id),
method: :delete,
data: { confirm: 'Are you sure?' } %>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This link doesn't work! You probably wanted passenger_path(@passenger.

Comment thread app/models/trip.rb
# use self because we are addressing behavior not attribute. The trip object is passed into the function and then the cost is returned.
def self.total_cost(input)
total = input.cost/100
return "%.2f"%total

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 disagree that these should be self methods. I think it is much clearer to say something like

@trip.total_cost

than

Trip.total_cost(@trip)

Comment thread app/models/trip.rb
def self.total_cost(input)
total = input.cost/100
return "%.2f"%total
end

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You seem to have written this method twice.

Comment thread app/models/trip.rb
def self.total_earning(trip)
total = ((trip.cost - (trip.cost * 0.15)) /100)
return "%.2f"%total
end

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Again, you seem to have this method twice. Mis-merge?

<% if counter == 0 %>
<%= "Driver does not have a rating yet. " %>
<% else %>
<%= "Average Rating: " %><%= average_rating/counter %>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This logic is a great candidate for moving to a model method. Instead of keeping a counter and doing math in the view, you ought to be able call @driver.average_rating.

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.

3 participants