MichaelSid/Week4-Test

PASS

Opened this issue · 1 comments

Filenames do not match class names

I think

result == self.first ? (self[1..-1].each {|x| result=yield(result, x)}) : (self.each {|x| result=yield(result, x)})

can be written as, make sure you fully understand what's going on here.

self.each {|x| result=yield(result, x)}

Fix your editor, be consistent with either tabs or space do not mix the two.

Good use of separation between text message and takeaway. Try and make the instance generic and the implementation specific. Imagine switching to a different text message provider would the variable name twilioClient make sense in the Takeaway class?

It's a small thing but naming is really important

Takeaway.new(TwilioClient.new)

class Takeaway
  def initialize(textMessageClient)
    ...
  end
end

Notice the constructor for the Takeaway class can take any kind of textMessageClient. In this case we provide it with an instance of TwillioClient.

Have a read of http://www.objectmentor.com/resources/articles/srp.pdf to reinforce understanding.

Thank you! I don't really get your first point about re-writing this:

self.each {|x| result=yield(result, x)}

Would be great to talk about it later. Thanks

On Wed, Mar 19, 2014 at 9:11 AM, Antony Denyer notifications@github.comwrote:

Filenames do not match class names

I think

result == self.first ? (self[1..-1].each {|x| result=yield(result, x)}) : (self.each {|x| result=yield(result, x)})

can be written as, make sure you fully understand what's going on here.

self.each {|x| result=yield(result, x)}

Fix your editor, be consistent with either tabs or space do not mix the
two.

Good use of separation between text message and takeaway. Try and make the
instance generic and the implementation specific. Imagine switching to a
different text message provider would the variable name twilioClient make
sense in the Takeaway class?

It's a small thing but naming is really important

Takeaway.new(TwilioClient.new)

class Takeaway
def initialize(textMessageClient)
...
end
end

Notice the constructor for the Takeaway class can take any kind of
textMessageClient. In this case we provide it with an instance of
TwillioClient.

Have a read of http://www.objectmentor.com/resources/articles/srp.pdf to
reinforce understanding.

Reply to this email directly or view it on GitHubhttps://github.com//issues/1
.