Skip to content
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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

takeaway-challenge #2234

wants to merge 2 commits into from

Conversation

laura-voss
Copy link

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).

  • User story 1: "I would like to see a list of dishes with prices"
  • User story 2: "I would like to be able to select some number of several available dishes"
  • User story 3: "I would like to check that the total I have been given matches the sum of the various dishes in my order"
  • User story 4: "I would like to receive a text such as "Thank you! Your order was placed and will be delivered before 18:52" after I have ordered"

README checklist

Does your README contains instructions for

  • how to install,
  • how to run,
  • and how to test your code?

Here is a pill that can help you write a great README!

* 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
Copy link

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
Copy link

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

Copy link

@eoinmakers eoinmakers left a 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
Copy link

@eoinmakers eoinmakers May 6, 2022

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]}" }

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

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)

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.

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