-
-
Save IanWhitney/5e2f3ff7099768f3cf40 to your computer and use it in GitHub Desktop.
class Bob | |
def reply_to(statement) | |
public_send("reply_to_#{statement.class}".downcase.to_sym) | |
rescue NoMethodError | |
default_reply | |
end | |
def reply_to_silence | |
"Fine. Be that way!" | |
end | |
def reply_to_yelling | |
"Woah, chill out!" | |
end | |
def reply_to_question | |
"Sure." | |
end | |
def default_reply | |
"Whatever." | |
end | |
end | |
class Statements | |
def self.all | |
[Question, Yelling, Silence, NullStatement] | |
end | |
end | |
Statement = Struct.new(:statement) | |
class Question < Statement | |
def self.match?(statement) | |
statement.end_with?("?") | |
end | |
end | |
class Yelling < Statement | |
def self.match?(statement) | |
statement.upcase == statement && statement.downcase != statement | |
end | |
end | |
class Silence < Statement | |
def self.match?(statement) | |
statement.strip.empty? | |
end | |
end | |
class NullStatement < Statement | |
def self.match?(statement) | |
true | |
end | |
end | |
class StatementFactory | |
def self.build(statement) | |
Statements.all.detect { |s| s.match?(statement) }.new(statement) | |
end | |
end |
@amcaplan I agree with you regarding #2 — I don't love the use of exceptions for flow control. However I think this is a contrived example with the stated goal of zero conditionals, so maybe we shouldn't judge too harshly.
However regarding your gripe in #1:
We've basically now forced Bob to know the names of lots of other classes.
I don't see how Bob
having a REPLIES
hash with keys matching the names of Statement classes is any different from Bob
having methods matching those names.
In addition, I don't love the Factory, nor the need to maintain an array of Statements in a separate class. Now if a programmer creates a new kind of Statement, she also needs to know about putting it into an array in a separate class. @mereghost's solution leaving instantiation to the superclass and using inherited?
to keep track of all descendants seems the ideal solution to me.
@mereghost's answer is pretty interesting but I think it has a typo?
I think it should be like this:
self.inherited(klass)
No question mark in the name.
@rmcsharry Yep. >.<
So many comments here...
I'll just say I have 2 major complaints here:
While I do think case statements are evil, and conditionals which lots of conditions are ugly, I don't think this is a very Ruby solution. It rails against Ruby's trusting and duck typing nature. We've basically now forced
Bob
to know the names of lots of other classes.I also don't love having the object send itself messages and rescue
NoMethodError
. Errors shouldn't be used for control flow, and sending messages is cool but there are better ways of doing this.So first, here's how I would handle it just addressing issue #2:
Simple and concise.
Next, I'd agree with @hehejo that the StatementFactory's job is to know what kind of Statement to build; Statements themselves are just data containers and should be pretty dumb. So I'd recommend something like this:
That looks oddly similar to a previous example... note, though, that we don't need a NullStatement, because
::type_for_statement
will return anil
type. Here's how we would use it in a slightly modified/simplifiedBob
:But we can do better. Here's a somewhat nicer version of
StatementFactory
:I think an if/elsif construct as above would be fine for
StatementFactory
, but if we expected to add more statement types, this gives us one place where we can add new types. We could use methods, but since they're all doing basically the same thing, I like having a hash where you can easily group and see the correspondence between the symbol representing a statement type, and a lambda representing the conditions that make it relevant.This solution does rely on Ruby maintaining the order of hashes, which it does in recent versions.
I prefer this solution because it's easily extensible, but doesn't overwhelm us with tons of classes. Data containers should contain data, not be data.
And it doesn't hurt that it's significantly more concise than the previously proposed solution. 😄