8 ms·
Tell, don't ask
- ninetax 14y agoAwesome! This is think kind of article I love to see on HN.
- fusiongyro 14y agoThis is a great little refresher. I recently inherited a codebase of about 3500 lines, 2500 of which are in one class. Not OO. Incidentally, if this article blew your mind or if you're interested in Smalltalk, I can't recommend this book enough: http://www.lulu.com/shop/andres-valloud/a-mentoring-course-on-smalltalk/paperback/product-3788890.html http://www.lulu.com/shop/andres-valloud/a-mentoring-course-o... The author has a radical take on OO in Smalltalk and shows that going to absurd lengths to eliminate if-statements (well, ifTrue: et. al., because it's Smalltalk) can lead to both better readability and better performance.
- Killswitch 14y agoWow, short article, but huge impact... I'm gonna start doing this... Makes so much more sense. Thank you.
- 0x0 14y agoGoing down this path seems to put your code at risk for dangerous mixing of concerns, in some cases. Unit-testing the check_for_overheating inside SystemMonitor looks complicated... The "sound_alarms" call inside probably needs to be a reference to a "Speaker.sound_alarms", right? Why should the SystemMonitor be locked to the API of a Speaker? etc.
- unconed 14y agoThe 'sound_alarms' call looks more like a local method to me, which could invoke the speaker in a loosely coupled fashion. The point is more about putting the behavior itself in the object. For unit testing, you'd want to move the individual tests into their own classes, letting the SystemMonitor be the glue that calls them and routes responses to the appropriate system.
- 0x0 14y agoYour second sentence pretty much hit the nail on the head, as I was reading your first: I'd create a different "controller" object that glues the SystemMonitor and the Speakers together. Then you'd have the neat little "tell, don't ask" in THAT object, instead. (Pretty much what you suggested)
- MaxGabriel 14y agoCool, I haven't heard of this before; I will think about it. Presumably, an exception would be 'view' type objects in an MVC setup?
- r00k 14y agoI apply this principle to view objects as well. Can you give an example of something you think should be excepted?
- MaxGabriel 14y agoI'm still thinking this through, here's what I came up with: If there's an on/off button to show additional information on the screen, it would seem weird to me to have the button (view) try and display the additional settings. The view has no idea what kind of environment it is in, how does it know if it can display the information without moving other things around. But a controller object managing all the views on the screen does. This also avoids subclassing or adding methods to the view.
- nigelsampson 14y agoYes and no, the approach I've used in MVC (.Net) is passing View Models to the View (rather than pure models). That means you can push some of the "tell" into the view model rather than having that logic in the view itself. I haven't done any RoR but I imagine a similar approach would work there.
- tbrownaw 14y agoThis principle applies much more broadly than just to programming with objects, for example it's an excellent way to deal with bureaucracies or managers who don't always read their email.
- unconed 14y agoExcellent. As a self-taught hacker, I always felt like I was missing something about OO, because it's introduced with an analogy to real world objects, while glossing over the more subtle contracts that programming objects have with each other. Seems like we need a more developed vocabulary to express these things.
- rahoulb 14y agoI first heard "Tell don't ask" in relation to Smalltalk. If you want a developed vocabulary, Smalltalk is where you find it (and it's been there for thirty-odd years).
- ninetax 14y agoWhat would you recommend? Smalltalk or Pharo?
- gnaritas 14y agoPharo is a Smalltalk, and yes, Pharo.
- rahoulb 14y agoI'd recommend "The Art and Science of Smalltalk" by Simon Lewis. Because it's so old, Smalltalk does things very differently to other development environments. The GUI is strange by today's standards (Smalltalk was why Xerox developed the GUI - you can see how much extra work Apple did to make the windows/icons/menus that we use today). And it uses an image (which is sort of like developing in a VM, and then copying that VM to another machine to deploy). So diving straight into a Smalltalk may leave you a bit lost - whereas the book is quite a good primer on OOP in general and where those OO patterns came from (whether Squeak, Pharo, GNU or one of the big expensive implementations).
- roguecoder 14y agoI think the vocabulary has been there, it's just very poorly taught/communicated. The c2 wiki was where I first encountered it. Old OOPSLA program proceedings, if you can find them, have some jewels among the cruft. And there is always Smalltalk Best Practice Patterns...
- jawr 14y agoI liked the first example, but then it felt too preachy; OOP isn't necessarily the correct way to do things. I really like the clean design of the site though, very fresh imo.
- djacobs 14y agoFor me, this technique is bigger than objects or encapsulation. It's about reusing existing "branch points" that a language gives you (whether it is polymorphism, method dispatch, namespacing) instead of explicit conditionals at a level higher than the language. My general take is that explicit conditionals in a high-level language are a smell. Sometimes they're necessary, but if you tell yourself that they mostly aren't, you tend to end up with cleaner code. I'm not saying it's possible to avoid conditionals completely, but this article gives several good examples of where it's possible. Someone should put together a similar set of examples in a functional language.
- 0x0 14y agoTo a certain level this is true. However if you hide all branching logic inside OO techniques, following the flow of logic and decisions becomes harder as they are hidden in deeply nested class hierarchies and overridden functions.
- gnaritas 14y agoThere's a saying about good OO code, everything happens somewhere else.
- moe 14y agoIt's a slippery slope, though. When you have a chunk of 50 consecutive lines with branches, then yes, that might be worth an abstraction. Sadly too often, in ruby, I see people turning 8 consecutive lines into 4 layers of indirection...
- gnaritas 14y agoThe slope flows the other way IMHO, most of the time most programs I've seen arent abstracting enough and the code is far to sequential and unnecessarily complicated because of it. Excessive if and switches seem far more common to me than excessive use of polymorphism.
- omgsean 14y agoIf you're interested in this kind of thing I highly recommend Avdi's Objects on Rails book (it's free) and Destroy All Software's screencast collection.
- bluesnowmonkey 14y agoExample 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
- zwieback 14y agoAlso see http://c2.com/cgi/wiki?TellDontAsk http://c2.com/cgi/wiki?TellDontAsk
- ricardobeat 14y agoDisclaimer: not a Ruby user. I'm confused by example 4. Why would you return a message from either of them? Shouldn't messages in general belong to the view? {{ user.address || "No address on file" }} The "not so good" code is essentially this, but inside a wrapper in view code. What if you need different markup for a missing address, you either stuff it into a method or change the method's return value to nil... and what if only street_name is missing, not the whole address? It looks like a big mess to me.
- randomdata 14y agoYour example is not functionally equivalent as address is a model instance with many address-related properties. Though I agree with your general premise. This, to me, seems like the Tell, Don't Ask solution: class User delegate :street_name, to: :address, prefix: true, allow_nil: true end <%= user.address_street_name || "No street name on file" %>
- lincolnq 14y agoMakes some good points. I can't help but notice, though, that the "good" examples are all longer. It makes me want to design a programming language that makes good code shorter than bad code...
- LukeShu 14y agoWith the exception of example 3 (which is possibly a bad example, see the discussion above), this is because of how much code is necessary to tell what's going on, rather than an increase in code size. Example 1: 5->1: It did get shorter. Example 2: 5->8: One of the lines is showing a line calling the check_for_overheating method, which wasn't shown in the first example. The other two are declaring the SystemMonitor class, which was declared "off-screen" before. Example 4: 7->13: Five lines for declaring and defining User.address, which was off-screen before. So it did get a line longer.
- n72 14y agoRelatedly, a long, long time ago I found Allan Holub's PPT Everything You Know is Wrong quite good: http://ebookbrowse.com/everything-you-know-is-wrong-pdf-d281291032 http://ebookbrowse.com/everything-you-know-is-wrong-pdf-d281...
- n72 14y agoIck, breaks command/query separation, the other OO rule that along with tell, don't ask I find to be a hallmark of well coded OO systems. class SystemMonitor def check_for_overheating if temperature > 100 sound_alarms end end end
- gav 14y agoThe first example isn't a good one. You don't want to have the user class responsible for returning the welcome message.
- sukuriant 14y agoThis has long been one of my favorite methods of using OO code to my advantage; and is one of the main reasons that my code is OO in the first place. There are many cases where it's easier/lazier to have if-statements. Going further, you can take the things learned in this article to make your general purpose code potentially faster, as well. For example, if a set of things that must be performed in order; and sometimes some of those steps are missing, instead of performing a null-check, you can have a default no-op case; that way, instead of: if(firstAction != null) firstAction(); if(secondAction != null) secondAction(); if(thirdAction != null) thirdAction(); in your inner loop, you can have: firstAction(); secondAction(); thirdAction(); While whenever firstAction, secondAction, thirdAction are set, you can say: firstAction = newFirstAction ?? noop; // etc. All in all, I'm glad this sort of knowledge is getting out there. I'm just grateful for my having really, really good CS teachers back in highschool ( I don't remember my college talking about OO as a way of removing if-statements ). * Users of Java, of course, will just need to write a NoOp instance of their interface to take advantage of this.
- cgmorton 14y agoI'm not clear on how that makes your code more maintainable though. The first case makes it explicit that any of the actions can fail to occur, whereas the second one, on first glance, seems to have all the actions occuring. I, as a newcomer to this code, will almost definitely make that mistake, which will make debugging or maintenance harder. The only way the second is even -as good- is if I'm constantly holding in my mind the various idioms you've used in the code, in this case that your function pointers are never null but will instead refer to an action that might do nothing. As far as I can tell, this is more work for me, for no gain. I would love to hear your reasoning why this way of doing it is -better-, as opposed to just -more object oriented- (or -marginally less typing-).
- sukuriant 14y agoI may expand on this later; but, whenever you're going to a new code-base, you're going to have to learn the various idioms that are at work in that code base. This is especially true when you're working with some more complicated languages where no one uses the whole set of it (see: C++). When I'm writing my code, I personally find that being able to trust what my code is doing to be more readable. In the case I wrote above, more than likely, I would have arrived at that point by first writing whatever the first action was; and then coming to know that there could be two different actions that could have taken place (causing a method call/inheritance to occur) and then I realized that sometimes, nothing might happen. Now we have a nothing case. The nothing case did not negatively effect my code flow. I am not 100% sure I would write code like I had above in the first place, it would grow to that state organically; but, the advantage of trust later on, was worth noting. I originally heard this concept a long, long, long time ago; and one of the interesting selling points that the person that told me it was that code could be faster run if it had no if-statements, reason being branching and branch prediction forces the processor to rewind; whereas a guaranteed jump is potentially less expensive, especially if the code is in the cache. Of course, this would be a premature optimization, but if the code occurred in the inner-most loop, there may be some gains to be had that otherwise wouldn't be. I suppose, for me, it looks cleaner when you're dealing with larger projects. That said, as I contemplate it further, I could certainly see where it would slip up some people, especially newcomers to my code. This sort of creativity would probably primarily spring up in organic/fluid code where OO paradigms are already in place. Thanks for making me think on this further :)
- InclinedPlane 14y agoRefactoring: http://www.amazon.com/Refactoring-Improving-Design-Existing-Code/dp/0201485672/ http://www.amazon.com/Refactoring-Improving-Design-Existing-... Read it. Learn it. Love it. Make it a habit that is so ingrained you do it automatically. Extract method: do it reflexively.
- afrenchcoder 14y agoall of those examples look horrible, make debugging harder and implies strong code coupling. example 1, we're mixing data with UI labels. How do you handle localization ? by coupling your localization code with your user data/behavior ? example 2 : you're simply coupling the system_monitor with the alarm, while in the worst case, the alarm should be linked to the system_monitor. Now if you want to add a "report_to_government_agency" method, you'd add that inside the code of every single one of your monitors (knowing that you don't want to report a broken light, but it might be a good idea for a melting nuclear core) ? Note that I'm not saying that the first code is good, it's just as bad... Also, the method becomes very poorly named (I want to know if the sensor went back to normal, but every time I query "check_for_overheating", it just rings the alarm and doesn't give me any info back ???) example 3 is just a poor usage of pseudo inheritance (and an abuse of duck typing). you create a new type of user for every messaging service again ? and if a user uses more than one messaging service, you just create a type for every combination ? Not very scalable nor readable, IMO. The last example is just as bad. useless inheritance, senseless object. the definition of street_name is wrong. the doc will be around "Street name returns a street name or an error message if there's no street name defined" How do you know if there's no street name, now ? All those examples basically make all extension harder. They're everything that's wrong with OO. an object is not about one structure that contains data and does everything that can be done with it. An object is about giving organized access to pertinent data and/or pertinent behavior and (potentially) allowing to change them.
- ybother 14y agoOr how to make code that is easy to exploit. This teaches me to avoid ruby, if anything.
- robotment 14y agoThe example is too simple to describe this design.
- keymone 14y agoexamples could be much better..
- edoloughlin 14y agoI'm not trying to be elitist or superior, but I can't understand why this has spent such a long time on the front page. Shouldn't the content of the article be considered pretty basic knowledge at this stage of OO? Deferring implementation specifics using polymorphism was one of the first things I learned about OO when I was introduced to it (via C++) around 1992. It was definitely 'aha' worthy then, when the vast majority of commercial development was procedural, but these days? Shouldn't it be considered a basic tool in most devs' repertoire?
- djacobs 14y agoIt should be, but I can assure you that it's not. Also, I think this has much less to do with OO than with conditionals at large.