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

Gawain Hewitt Takeaway challenge #2235

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions lib/menu.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,13 @@ def show
end

def check(dish)
_dish = dish.to_sym
the_dish = dish.to_sym
@dishes.each do |dish|
"food #{dish[:food]}"
if _dish == dish[:food]
if the_dish == dish[:food]

Choose a reason for hiding this comment

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

I feel this could be simplified to a ternary operator

ie 'the_dish == dish[:food] ? true : false

Copy link

Choose a reason for hiding this comment

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

Maybe this method could also be renamed to something like contains_dish? so it's more explicit on what it does

return true
end
end
return false
end

end

57 changes: 38 additions & 19 deletions lib/takeaway.rb
Original file line number Diff line number Diff line change
@@ -1,16 +1,14 @@
require './lib/menu'

class Takeaway

attr_reader :menu, :summary

def initialize(input: $stdin, output: $stdout)
@input = input
@output = output
@menu = Menu.new

Choose a reason for hiding this comment

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

I'm impressed you definitely know what you're doing, especially with the corresponding tests

I'm no expert in dependency injection, but this article near the beginning

https://medium.com/@Bakku1505/introduction-to-dependency-injection-in-ruby-dc238655a278

explains how linking the classes together so "strongly" ie 'def initialize... @menu = Menu.new' can cause issues down the line

@dishes = []
end

def menu
@menu
@summary = []
end

def show_menu
Expand All @@ -19,24 +17,45 @@ def show_menu

def order
loop do
Copy link

Choose a reason for hiding this comment

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

Nice that you've worked with user input as well! I'd suggest looking at extracting further this logic into a new class, maybe named Cli or Interface, which would contain methods such as clarify_order or ask_for_order. It could probably also handle the display of the menu (which is right now in the Menu class). This way you can make the main Takeaway and Menu classes do a bit less, and it also makes it easier to test the input/output, as there will be only one class that directly uses puts or gets, so you remove the need to mock these for the other classes

@output.puts "Please type each dish you require followed by return. When you have finished your order press return twice."
dish = @input.gets.chomp
# return true if dish == ""
dish = ask_for_order
return true if dish == ""
if menu.check(dish)
@output.puts "how many do you want?"
quantity = @input.gets.to_i
return true
quantity = ask_for_quantity
log_order(dish, quantity)
else
@output.puts "Sorry, we don't have that dish - perhaps you've made a spelling mistake?"
clarify_order
end
end
end

end

private

def confirm_order
true
end

# need to get the method order looping to keep asking until double return
# line 24 and removing line 28 does this for an irb test but it breaks
# the rspec tests. Until this is fixed we can't run the 3 items test
# also the order is not being put anywhere yet - needs to be added to a
# hash - and perhaps to and order class
def ask_for_order
@output.puts <<~ORDER
Please type each dish you require followed by return.
When you have finished your order press return twice.
ORDER
dish = @input.gets.chomp
end

def clarify_order
@output.puts "Sorry, we don't have that dish - perhaps you've made a spelling mistake?"
end

def ask_for_quantity
@output.puts "how many do you want?"
quantity = @input.gets.to_i
end

def log_order(dish, quantity)
order_item = {}
order_item[:food] = dish
order_item[:quantity] = quantity
@summary << order_item
end

Choose a reason for hiding this comment

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

Really impressive thought about actual user interface, certainly going beyond what is required. You have my sympathy and respect for keeping your head around it though

end
12 changes: 8 additions & 4 deletions spec/feature_test_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,21 @@
output = place_order_with_input("Brocoli", "Broccoli")
Copy link

Choose a reason for hiding this comment

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

Nice one! This test suite should probably contain a bit more tests, at least one testing the whole integration of the different components with a successful order process, and assert the output of the takeaway.order method


expect(output).to eq <<~OUTPUT
Please type each dish you require followed by return. When you have finished your order press return twice.
Please type each dish you require followed by return.
When you have finished your order press return twice.
Sorry, we don't have that dish - perhaps you've made a spelling mistake?
Please type each dish you require followed by return. When you have finished your order press return twice.
Please type each dish you require followed by return.
When you have finished your order press return twice.
how many do you want?
Please type each dish you require followed by return.
When you have finished your order press return twice.
OUTPUT
end
end

def place_order_with_input(*order)
input = StringIO.new(order.join("\n") + "\n")
output= StringIO.new
input = StringIO.new(order.join("\n") + "\n" + "\n" + "\n")
Copy link

Choose a reason for hiding this comment

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

Really nice way to mock IO!

output = StringIO.new

takeaway = Takeaway.new(input: input, output: output)
expect(takeaway.order).to eq true
Expand Down
1 change: 0 additions & 1 deletion spec/menu_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,3 @@
end

Choose a reason for hiding this comment

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

your general ruby syntax and string-morphing is fantastic!

end

51 changes: 40 additions & 11 deletions spec/takeaway_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,31 +11,49 @@
end

context '#order' do
it 'can order one item from menu' do
output = place_order_with_input("Broccoli")
it 'can ask for one order from menu' do
output = place_order_with_input_and_return_output("Broccoli")

expect(output).to eq <<~OUTPUT
Please type each dish you require followed by return. When you have finished your order press return twice.
Please type each dish you require followed by return.
When you have finished your order press return twice.
how many do you want?
Please type each dish you require followed by return.
When you have finished your order press return twice.
OUTPUT
end
xit 'can order three items from menu' do
output = place_order_with_input("Broccoli", 2, "Chips", 1, "Ice_cream", 1)
it 'can ask for three orders from menu' do
output = place_order_with_input_and_return_output("Broccoli", 2, "Chips", 1, "Ice_cream", 1)

expect(output).to eq <<~OUTPUT
Please type each dish you require followed by return. When you have finished your order press return twice.
Please type each dish you require followed by return.
When you have finished your order press return twice.
how many do you want?
Please type each dish you require followed by return. When you have finished your order press return twice.
Please type each dish you require followed by return.
When you have finished your order press return twice.
how many do you want?
Please type each dish you require followed by return. When you have finished your order press return twice.
Please type each dish you require followed by return.
When you have finished your order press return twice.
how many do you want?
Please type each dish you require followed by return.
When you have finished your order press return twice.
OUTPUT
end
it 'stores an order of one dish' do
takeaway = place_order_with_input_and_return_class("Chips", 1)
expect(takeaway.summary).to eq([{ food: "Chips", quantity: 1 }])
end
it 'stores an order of four dishes' do
takeaway = place_order_with_input_and_return_class("Chips", 1, "Broccoli", 3, "Tofu", 1, "Ice_cream", 4)
expect(takeaway.summary).to eq([{ food: "Chips", quantity: 1 },
{ food: "Broccoli", quantity: 3 }, { food: "Tofu", quantity: 1 },
{ food: "Ice_cream", quantity: 4 }])
end
end

Choose a reason for hiding this comment

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

again, already thinking about testing injection (looking at commit 1) here is very switched on. nice. took a lot of research/cheating for me to come to tests like this

def place_order_with_input(*order)
input = StringIO.new(order.join("\n") + "\n")
output= StringIO.new
def place_order_with_input_and_return_output(*order)
input = StringIO.new(order.join("\n") + "\n" + "\n" + "\n")
output = StringIO.new

takeaway = Takeaway.new(input: input, output: output)
allow(takeaway.menu).to receive(:check).and_return(true)
Expand All @@ -44,4 +62,15 @@ def place_order_with_input(*order)
output.string
end

def place_order_with_input_and_return_class(*order)
input = StringIO.new(order.join("\n") + "\n" + "\n" + "\n")
output = StringIO.new

takeaway = Takeaway.new(input: input, output: output)
allow(takeaway.menu).to receive(:check).and_return(true)
expect(takeaway.order).to eq true

takeaway
end

end