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
13 changes: 6 additions & 7 deletions lib/menu.rb
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
class Menu
attr_reader :contents

def initialize
@contents = [{food: :Chips, price: 1},
{food: :Tofu, price: 2}, {food: :Broccoli, price: 1},
{food: :Ice_cream, price: 2}]
@dishes = [{ food: :Chips, price: 1 },
{ food: :Tofu, price: 2 }, { food: :Broccoli, price: 1 },
Copy link

Choose a reason for hiding this comment

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

Minor one here - it's probably better to leave the dish names as strings rather than symbols, as symbols are mostly used for keys, special configuration values, enumerated values, etc, rather than purely "textual" values

{ food: :Ice_cream, price: 2 }]
end

def show
@contents.each_with_index do |item, index|
puts "#{index+1}. #{item[:food]} - £#{item[:price]}"
@dishes.each_with_index do |item, index|
puts "#{index + 1}. #{item[:food]} - £#{item[:price]}"
end
end

end

14 changes: 11 additions & 3 deletions lib/takeaway.rb
Original file line number Diff line number Diff line change
@@ -1,13 +1,21 @@
require './lib/menu.rb'
require './lib/menu'

class Takeaway
attr_reader :menu
def initialize
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 show_menu
@menu.show
end

def order
@output.puts "Please type each dish you require followed by return. When you have finished your order press return twice."
@dish = @input.gets.chomp
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
end
17 changes: 5 additions & 12 deletions spec/menu_spec.rb
Original file line number Diff line number Diff line change
@@ -1,18 +1,11 @@
require 'menu'

describe Menu do
it 'can hold dishes and prices' do
expect(subject.contents[0]).to eq({food: :Chips, price: 1})
end

it 'should respond to contents' do
expect(subject).to respond_to(:contents)
end

it 'should display menu' do
expect do
subject.show
end.to output("1. Chips - £1\n2. Tofu - £2\n3. Broccoli - £1\n4. Ice_cream - £2\n").to_stdout
context '#show' do
it 'should display menu' do
expect { subject.show }.to output("1. Chips - £1\n2. Tofu - £2\n3. Broccoli - £1\n4. Ice_cream - £2\n").to_stdout
end
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
end
28 changes: 22 additions & 6 deletions spec/takeaway_spec.rb
Original file line number Diff line number Diff line change
@@ -1,15 +1,31 @@
require 'takeaway'
require 'stringio'

Choose a reason for hiding this comment

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

You'll have to clue me in on this


describe Takeaway do
it 'should respond to show menu' do
expect(subject).to respond_to(:menu)

context '#show menu' do
it 'should call #show from class instance of Menu' do
expect(subject.menu).to receive(:show)
subject.show_menu
end
end

it 'should call display method from Menu' do
expect(subject.menu).to receive(:show)
subject.show_menu
context '#order' do
it 'this is my test' do
output = place_order_with_input("Broccoli")

expect(output).to eq "Please type each dish you require followed by return. When you have finished your order press return twice.\n"
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)
output= StringIO.new

end
takeaway = Takeaway.new(input: input, output: output)
expect(takeaway.order).to eq input.string

output.string
end

end