-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
takeaway-challenge #2234
base: main
Are you sure you want to change the base?
takeaway-challenge #2234
Conversation
* If you refer to the solution of another coach or student, please put a link to that in your README | ||
* If you have a partial solution, **still check in a partial solution** | ||
* You must submit a pull request to this repo with your code by 9am Monday morning | ||
* takeaway currently doesn't allow dishes to be deleted from order -> needs to be updated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a good idea - wish I'd thought of this!
|
||
def add_dish(number, qty) | ||
fail "please enter a valid dish number" if @menu.existing_dish(number, qty) != true | ||
if duplicate_dish(number, qty) == true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like a good idea. I just kept adding to the 'basket' array of hashes, so your idea of actually editing a quantity value seems a lot more like a real user experience
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a good implementation of Takeaway, well done!
My comments propose simpler solutions, as I think the use of the structure [{}, {}, {}] for the @menu made interacting with it in your code, and I imagine making changes to your code, challenging in places.
You achieved the functionality of all of the user stories.
There are some additional features, so while you should feel free to have fun with these challenges and try things out, remember when doing tech tests in the future that the interviewer wants to see you can work to a specification and not do more than is asked.
I thought there were a good range of unit tests included - including the tests that only check if certain methods are being called.
{ :item => 3, :dish => "Raita", :price => 3.50 }, | ||
{ :item => 4, :dish => "Garlic Naan", :price => 3.50 } | ||
] | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hashes are most useful where you have a key value pair - eg. if the only information you needed to record was the item name and price, a simple hash could look like:
@menu = { "1. Mattar Paneer" => 12.50, "Black Daal" => 7.50 }
@menu[item_name] will return the cost of that item.
For data requiring more than two values, where key/value isn't enough, I would suggest defining a new object, for example:
class Dish
def initialize(name, price)
@name = name
@price = price
end
# getter methods for retrieving information
end
This way, you could work with an array of Dishes inside your Menu class, eg. @menu = [Dish.new(args), Dish.new(args), etc.]
Singular dish data can be accessed with @menu[0].name, etc. Which can be quite a bit easier than traversing a complex hash!
end | ||
|
||
def view | ||
@menu.map { |dish| "#{dish[:item]}. #{dish[:dish]} £#{dish[:price]}" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Were you to use just a single key/value hash as the one I suggested in my first comment, you could refactor this to something like:
puts "#{key}:#{value}"
This will print the full menu.
def dish_added_message(number, qty) | ||
(@menu.selection(number, qty)).map do |dish| | ||
"#{qty} x #{dish[:dish]} £#{dish[:price]}/each, has been added to your order -> £#{(sub_total(number, qty))}" | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although this is functional, the functionality to receive a confirmation for each item added to the order is not mentioned in the spec - you should stick as closely as possible to the spec as possible. This helps keep your code clean / simple!
end | ||
end | ||
|
||
def sub_total(number, qty) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could omit this and only calculate the total at runtime / when the user confirms the order.
Laura Voss
Please write your full name here to make it easier to find your pull request.
User stories
Please list which user stories you've implemented (delete the ones that don't apply).
README checklist
Does your README contains instructions for
Here is a pill that can help you write a great README!