4 ms·
Example 3 adds a nonsense method, EmailUser#send_to_feed. What does that mean? Email users don't have feeds. If we're going to evangelize OO purity, let's do
by bluesnowmonkey 14y ago
Example 3 adds a nonsense method, EmailUser#send_to_feed. What does that mean? Email users don't have feeds. If we're going to evangelize OO purity, let's do it right.
class Post
def created
user.post_created(self)
end
def send_to_feed(feed)
feed.send(contents)
end
end
class TwitterUser
def post_created(post)
post.send_to_feed(twitter)
end
end
class EmailUser
def post_created(post)
# no-op.
end
end
The post merely tells its user that it was created. Then the user can decide to do something else. All users know about posts, but only TwitterUser knows about feeds.
Honestly though, if I saw the not-so-good code, I'd leave it alone. It takes a pretty big justification to double the code for the same features.
- briandear 14y agoThe better code results if far fewer headaches down the line. Having had to refactor entire apps that have been broken because of incrementally added crap, I vote for good longer code over sloppy shorter code. Of course, this is highly dependent on the situation and application of course.
- klochner 14y agocarrying on, why make all user models define a no-op? class User def post_created(post); end end class TwitterUser < User def post_created(post) post.send_to_feed(twitter) end end class EmailUser < User end
- mguterl 14y agoInheritance is not always the right solution.
- dekz 14y agoEspecially when the examples are written in a language which has mixins as core functionality.
- regularfry 14y agoMixins are inheritance. When people say "prefer composition over inheritance," they don't mean mixins.
- halogen64 14y agoA better solution would be to just use Observers. One for email, one for twitter. The user shouldn't care about how to talk with these services.
- malyk 14y agoObservers are one of the worst possible solutions because they lie outside the purview of, well, everything in the system. You don't ever see them in the code. You don't know they are there. They are pieces of unicorn code that have side effects that you won't know about or see because they aren't "in the code". Horrible solution. Code should be simple and easy to understand. Observers add significant complexity and make your code vulnerable to unnecessary bugs because the code that acts on your objects is invisible to the normal control flow of the program and those who write or maintain it.
- namidark 14y agoThey add (at least in Ruby) a few lines of code to a program; and take things like Email out of the User model and put them in a more appropriate place.
- malyk 14y agoAnd you'd never know that they existed if you look at the user model... My stance is that things like sending email should be explicit calls so they are obvious. Sending email when registering a new user, for example, should be in the if user.save branch in your controllers or, if you have a more SOAish app, in the service that creates users. def create user = User.create(params[:user]) if user.save Emailer.new_user_email.deliver render 'welcome' else render 'oh shit' end end Or, refactor that out a bit (ONLY IF NECESSARY!) def create user = UserService.create_user_from(params[:user]) if user render 'welcome' else render 'oh shit' end end
- skybrian 14y agoThis is all nonsense anyway. There's no such thing as a TwitterUser or an EmailUser. You just have users, some of whom use Twitter, some use email (and may have multiple addresses), and some use both, and they can edit their settings to add and remove accounts and change notification preferences. So it's really has-a rather than is-a. Adding unnecessary inheritance is far worse than the original problem.
- hajrice 14y agoI completely agree with you. I actually find the 'cleaner version' much much harder to read. Every time I look at the code I have to 'recompile' it in my brain, just because it's doing such a simple thing in such a darn complex way.
- ed_blackburn 14y agoI suspect the author is trying to use trivial examples to highlight the pattern. As with everything it's about judgement. If you're going to inherit another type be sure to observe Liskov Substitution Principle, otherwise in avoiding conditionals you're introducing another approach that will lead you towards entropy. That is after all what many of these principles are for isn't it? reduce entropy. Writing the code is easy, maintaining and adding crazy new features from the pesky business is where it gets expensive? Entropy kills apps. In my experience observing (pragmatically, never dogmatically) OO principles like SOLID reduces entropy. Thus are worth applying. Personally I prefer polymorphism and null object pattern over conditionals to deal with edge cases, though if you have a leaky abstraction in the first place no principle or pattern is going to save you!