11 ms·

How A Pull Request Rocked My World

320 points by 10char 14y ago

herge 14y ago
The classic "Replace Conditional with Polymorphism" design pattern, which took me way to long to realize that it also applies to Python.
- ghc 14y ago
  Indeed it does. I have a large code base at work that uses this pattern for Django models with a bit of metaprogramming to create a template for types of input sensors. It's amazing what this kind of refactor can do when you need to turn a long conditional into something more generally extensible. New sensor types with custom column configurations take an average of 5 lines of code now, instead of about 30 before.
- jonathanberger 14y ago
  Yes, it's cataloged in Martin Fowler's excellent book Refactoring, Improving the Design of Existing Code in Chapter 9. I'd recommend the book strongly.
  - herge 14y ago
    Link for the lazy: http://www.refactoring.com/catalog/ http://www.refactoring.com/catalog/
- deltasquared 14y ago
  Yup. I remember that particular realization. Now switch statements in OO languages just look broken to me. Thanks Python!
- Guillaume86 14y ago
  I like this kind of refactoring, make you feel like: http://thejoysofcode.com/post/36047317385/when-i-replace-200-lines-of-my-co-workers-code-with-10 http://thejoysofcode.com/post/36047317385/when-i-replace-200...
campnic 14y ago
I like Martin Fowler's Refactoring. This is called 'Replace conditional with subtype' explained here: http://www.refactoring.com/catalog/replaceConditionalWithPolymorphism.html http://www.refactoring.com/catalog/replaceConditionalWithPol... The book is a good read and a fantastic reference.
rasur 14y ago
IIRC, Fabio works at the Swiss Ruby shop Simplificator, who are all very nice chaps indeed! Glad he helped you along. EDIT: I just checked, yes he does (well, unless we're talking about another Fabio of course, which is possible...)
lincolnq 14y ago
I don't love this refactor. I especially don't like how it generates names from other names -- e.g., given 'switch'/'submit', appends '_row', and then converts it to CamelCase so you get 'SwitchRow'/'SubmitRow', then instantiates the class named that. Here's why: When I am maintaining somebody else's code, even if I guess that this is going on, I rarely trust these kinds of name-generation tricks. When I need to change some code, I'm going to grep the codebase for 'SwitchRow' and add a new line in whatever conditional to instantiate my new class. When grep can't find any instances of SwitchRow's constructor being called, I become suspicious and it costs me more time, because I am making sure I properly understand what is actually going on.
- rosser 14y ago
  ...I rarely trust these kinds of name-generation tricks. So you're probably not a big fan of Rails, then?
  - hnewser1 14y ago
    and what does `trust` mean? You don't trust that correct object won't be given back to you?
    - andreasvc 14y ago
      I suppose more like trust whether the resulting code will always be correct.
  - diminish 14y ago
    what about using a multiline YAML text to be loaded with automatic generation of objects from class name strings? (reminder!)
  - potatolicious 14y ago
    I'm actually not a huge fan of implicit name-generation tricks, but I'm also not entirely opposed to them. The trick here I think is consistency - is name-generated type dispatch the standard across the entirety of the codebase? If it is, go wild. If it's not then I'd be much more wary of it. Consistency and building correct expectations for future maintainers is pretty important. Doing a smart trick in one place but failing to do it in other places where it makes sense IMO tends to create a bigger mess than doing it the dumb-but-safe way everywhere.
laureny 14y ago
tl;dr: polymorphism is usually better than if/else.
- bfwi 14y ago
  I like this explanation: http://stackoverflow.com/questions/234458/do-polymorphism-or-conditionals-promote-better-design http://stackoverflow.com/questions/234458/do-polymorphism-or...
mquander 14y ago
I don't agree with this change. Based on the code, it looks like there should be an enumeration of cell_types somewhere, a button should have a cell_type, and then there's a map from cell_type to construction_fn (or a polymorphic set of classes to serve as the map.) Metaprogramming is something you do when you can't do it easily in the usual way. If a dispatch table or inheritance takes care of something, use the existing language features instead of building your own. (EDIT: On examination, there already is a RowType, and the pull request is a lot closer to my suggestion than I first thought, so mostly ignore this.)
jtchang 14y ago
Refactors like this are super cool and often needed but I sometimes have to think why we don't store the easier to read code as well. I know we have git and we can always do a diff to see what changed. What I am talking about is a more holistic view. Being able to search a code base for SwitchRow and then somehow the code comes up where it was refactored into this dynamic programming style. Git can do this but only if you are actively looking for it. I wish you could do a metacomment on the refactor without cluttering up the code.
- shocks 14y ago
  Because if you understand the new code you don't need the "easier to read" code. Constraining yourself to only writing code a beginner can understand is stupid. Maintaining good code and "easier to read for a beginner" code in tandem is a waste of man hours.
stickydink 14y ago
The article is very humble, but in my view, seems to go a little too far. "You can reuse code with inheritance. Absolutely crazy. Brilliant. All of this blew me away." Honestly?
- revelation 14y ago
  Thats a very critical point that you just have to understand, to "get". Many people know and learn about object oriented design, but when tasked to actually write a program based on these principles they will often revert to code where they make distinctions based on the type of what they have been handed. TLDR: dynamic_cast is bad design
  - mylittlepony 14y ago
    I guess he's gonna come back with another article when he realizes composition > inheritance.
    - alttab 14y ago
      You'll need to wait like 5 years if he was anything like me
    - deleted 14y ago
      [deleted]
  - to3m 14y ago
    Using static conditions rather than polymorphism seems to just be a common thing that people tend to do, regardless of skill level or quantity of experience. It's not uncommon for people just not to think of using polymorphism, if the objects in question wouldn't really model any particular kind of object as such (they're just there to provide a table of function pointers). This says nothing about the quality of their code, or the general OOness of their designs, or the number of digits in their IQ. It's a bit of a shame, because I've found code to be reliably improved by using objects as simple tables of function pointers. The same is less often true for using objects as models of things.
juddlyon 14y ago
Evocative writing for a technical post "serpentine" if/else, "rickety door" - I know nothing about iOS and enjoyed reading it.
guylhem 14y ago
On the first glance it seems great, but much more complex to understand and debug. The if tree was certainly not pretty, but straightforward - it did its thing. It was something that could be given to an average developer to work on and improve. The refactored code will require someone that's not just an average developer - if only because you need to read the article to understand it if you're not familiar with javascript, while the initial code needed only a glance to understand what it did. IMHO, that's just complexity for complexity sake - unless you have plans to take advantage of the new flexibility it provides. (and if you have to find, hire and pay that 'better than average' person, it might not be such a good thing.)
- hnewser1 14y ago
  This is not written in javascript, it is ruby. And I think your developer is way below average if he can't understand the pattern used in the refactored code.
  - dubcanada 14y ago
    If you have a ruby programmer who can't understand this, fire them. And get a new one. This type of stuff is all over ruby code.
iamdave 14y ago
I'm not a developer, but I read this article anyway. All of this blew me away. Like this "@mordaroso" fellow strolled on up, cocked his knee skyward, and jettisoned his leg into the rickety wooden door that previously sheltered my mind. That was just great writing.
- mikeklaas 14y ago
  I have to disagree. "Jettison" does not fit the simile he was going for, at all.
  - alecdbrooks 14y ago
    It's not great, but it could be good. Replace "jettison" and cut the cliched "All of this blew me away" and the resulting paragraph is descriptive and fairly tight.
  - deleted 14y ago
    [deleted]
- drakeandrews 14y ago
  Jettisoning ones leg into a door sounds painful. Unless one has a prosthetic, then I must wonder why one is throwing it at a door. Do they not need it to stand?
- cpeterso 14y ago
  I disagree. I had to parse the sentence three times to make all the pieces fit. :\
wcoenen 14y ago
This google tech talk is basically about the same thing, eliminating conditionals with polymorphism: https://www.youtube.com/watch?feature=player_embedded&v=4F72VULWFvc https://www.youtube.com/watch?feature=player_embedded&v=...
greenyoda 14y ago
It's scary to think that this guy who was just shocked to discover a very basic concept of object oriented programming (polymorphism) has written his own book on iOS programming: http://clayallsopp.com/posts/writing-a-programming-book http://clayallsopp.com/posts/writing-a-programming-book
- AlexeyBrin 14y ago
  How many books did you wrote ?
  - mylittlepony 14y ago
    How is that relevant?
    - AlexeyBrin 14y ago
      Just pointing that his little ad hominem attack has no place on HN.
      - mylittlepony 14y ago
        It would be an ad hominem if the book was about gardening but it's not, it's about programming.
  - adamnemecek 14y ago
    Ah, the good ol' ad hominem.
  - icebraining 14y ago
    That's not an Ad Hominem.
    - adamnemecek 14y ago
      I assume you are responding to me. Yes, yes it is. From Wiki "Argumentum ad hominem, is an argument made personally against an opponent instead of against their argument." AlexeyBrin is implying that because greenyoda may not have written any programming books, that his argument is less valid. AlexeyBrin's argument is not refuting greenyoda's central point, that it's somewhat strange that the blog post author has written a programming book about Ruby, despite not being aware of what some argue are the language's commonly used features.
svachalek 14y ago
In terms of Gang of Four patterns, I believe this would be Strategy + Factory Method. The book is a little antiquated (1994) but it or a better successor should be required reading for any programmer.
- chris_wot 14y ago
  I don't believe that its terribly antiquated. Pretty much everything bases itself off of this book. Edit: drafted iPad - fixed spelling. Trilby - does anyone even use that word anymore?!?
  - martinced 14y ago
    "I don't believe that it's the trilby antiquated. Pretty much everything bases itself off of this book." Hardly. The GoF book is sweet if in this day and age you're still doing Java/C# but if you're programming in a language like, say, Clojure you can throw about 75% of the GoF book into the trash. pg said to use a Lisp to beat the average and that's what I'm doing. I'm certainly not accepting the status-quo and considering that, say, the "visitor pattern" is a godsend. Most of these "patterns" only make sense in a non-functional and OO language and are only needed because of missing languages concepts (like the lack of high-order function and/or the lack of real macros).
    - chris_wot 14y ago
      I'll be honest - I'm not terribly familiar with functional programming languages.
    - cpeterso 14y ago
      Design patterns are, admittedly, workarounds for language deficiencies, but the book is still recent because far more people program in Java and C# than Clojure.
    - svachalek 14y ago
      I don't think Clojure is really object-oriented, and therefore wouldn't recommend the use of object-oriented design patterns at all. But as the dominant paradigm of our age, I think it's worth understanding OOP even if the final decision is to reject it.
Xcelerate 14y ago
Reading the comments in this thread is very interesting to me, because each programmer seems certain of the superiority of his own programming style. We have some comments that disparage the change, saying that it obfuscates the code and makes it more difficult to understand. We have others saying that this change is a basic technique and it's "shocking" that the author wrote a book without this simple knowledge. What I take from this is yet another situation of people arguing about something highly subjective that is closely tied to their identity. Good programming is not an objective concept. People respond differently to different programming techniques and what works better for one person may greatly confuse another.
- AlexeyBrin 14y ago
  +1 for Good programming is not an objective concept.
  - nikster 14y ago
    I dont agree with that. There is a confusion about programming though, an idea that it is a purely technical discipline, like laying bricks when in reality its a design discipline, like designing a building. If you wanted to build a house you wouldnt just hire a couple of bricklayers and tell them "do it". Yet thats what happens in programming...
  - dakimov 14y ago
    == Programming sucks nowadays. There is not even a consensus on what is good in programming! Take a look at other professional cultures: people work extremely hard to develop the skills that everybody else in their profession have. What does a software developer do? Whatever they like on their own, there is no professional culture whatsoever: "Don't give a crap about the humankind's experience, I'll invent everything on my own the way I like". This is actually unbelievable how different programming is in this aspect from other disciplines. I don't even mentor Junior Developers because of that, as I can't refer to anything authoritative and say "Do it like this" because there is always a bunch of folks that do it the opposite way. There is almost nothing to rely on.
sbuccini 14y ago
To be honest, I'm very disappointed with the comments here. Clay is obviously a very talented programmer, and just like most talented programmers, he didn't start out that way. This article isn't about how he learned OOP, it's about how someone opened his eyes to how code can be elegant instead of simply just functional. This article hits home with me especially. I just learned Python last semester in my intro to CS class. I had an eerily similar moment when I learned how to use generators. "WOW! You can simplify this huge block of code into one line! It's..beautiful." It's a really special moment, and I'm glad Clay shared his.
leriksen 14y ago
Those who know this technique either discover it themselves, through "reading, learning, doing", or are shown,like this guy was. None of us was born knowing this. Clay has blogged about his experience of discovering this - that in itself is what makes this a great read for me. I wouldn't be surprised if every single tech book author, at some point after their book has been published, has learned something and thought "I wish I knew that when I wrote my book !!" By their very nature books document the authors understanding at a point in time. Would polymorphism have helped Clay's book ? Probably. Does not knowing it ruin the book ? Probably not. If we waited 'til we knew everything before we wrote a book about anything, nothing would ever get written.
- AlexeyBrin 14y ago
  Writing a book is also a great way to discover new things about your subject.
  - nirvdrum 14y ago
    While true, when I purchase a book I'm trusting that the author is an expert in the topic. Maybe my expectations are too high.
    - chris_wot 14y ago
      You may never know - if it teaches you some things and makes you more productive, then what of it?
      - nirvdrum 14y ago
        My concern is the inverse of that: recommending something you either don't fully understand or haven't used long enough to speak to either its pros or cons. Being in a book usually is an indicator that something is a best practice or how the way things should be done, ostensibly being put forth by an expert. Perhaps we shouldn't hold such content to such a standard.
chaz 14y ago
Critiquing these particular lines of code or the pull request is unimportant. My takeaway was that it's hugely valuable to simply make your code available and have other people look at it. Not only can it improve your own code, but you can personally learn something new from other people. You can be sure Now I'm feeling a bit guilty for not having open sourced any of the code I've ever written, and I'm wondering what kinds of lessons I could have learned earlier on.
rckrd 14y ago
I would like to point out that he was able to describe his original implementation in a few sentences. This is important.
jules 14y ago
Novice programmer: Uses simple if/else. Adept programmer: Discovers polymorphism. It's great, uses it for everything. Master programmer: Realizes that in many cases, it makes the code less clear and more verbose, reverts back to if/else until the flexibility is actually needed. Same story repeats itself many times on different fronts: metaprogramming, recursion, exceptions, macros, etc.
- dougk16 14y ago
  One more step: Master programmer realizes that they're still a Novice programmer - repeats the cycle.
rachelbythebay 14y ago
I tried to picture the solution before scrolling down past the initial block with the nested ifs. It got me thinking about "switch()", as seen in C, C++, and elsewhere. That in turn got me thinking about using an enum to pick up on whatever it is he's trying to do here, and jump to the right place accordingly. Generically, I had something like this in my head: switch (row_type) { case submittable: ...; break; case checkable: ...; break; ... } Then I started realizing that using a switch means you have isolated things such that they can only be one of the n types. It won't ever be submittable AND checkable, for instance. I assume that's what is desired here: mutual exclusivity for all of these possibilities. Looking back at the original code, I see that by using the nested if/else construct, it effectively gives priority to them in the order in which they are encountered. A row with a submit_button is going to fire off make_submit_cell() whether or not it has checkable set. It trumps the possible call to make_check_cell, in other words. These two approaches solve two different problems. Without knowing more, it's hard to say whether the "priority" approach makes sense here. If I had to guess, based on the refactoring which turned it into a series of distinct classes, that sounds like it should have been "pick 1 of n" (one-hot code) all along. If that's so, the if block wasn't just ugly, but it was actually covering up some real problems. It was just asking for trouble in terms of bugs down the road since it allowed nonsensical settings. But that's just a guess. I could be wrong.
- potatolicious 14y ago
  Indeed, the original code was, IMO, incorrect. It looks like the purpose is a pretty common one in iOS - you have a table view populated by many rows, each of which can be a different class that renders and behaves differently. It's a common need in iOS apps. What he really needed was an enum, not flags. The choice of class is mutually exclusive and flags are horrible for tracking this, and are great for hiding bugs - one erroneous flag flip and suddenly your code is walking down a completely different path that's non-obvious on first debug. Using a bunch of bools to approximate state is error-prone and IMO just bad architecture.
jQueryIsAwesome 14y ago
I do something a little bit similar with DOM interaction in JS, for example button ".btn_pdf_docs" shows all ".pdf_docs" elements. I consider the code to implement this more readable than a bunch of events binding.
jamieb 14y ago
This person found something that they didn't know they didn't know and were so happy they blogged about it. Regardless of what I may personally think about the merits of the code before or after, it makes me happy that there is another human being who is thrilled by learning something new about programming.
deleted 14y ago
[deleted]
davidrudder 14y ago
I like it when developers praise each other.
vemv 14y ago
Fabio's approach does the job, but it fails at genericity - its usefulness is limited to the scope of the Formotion project. Which is unfortunate because something actually simple is going on - a mapping of keywords to functions. What you really want is multimethods - they provide all the goodness of classic polymorphism, without forcing you to model intrincate, rigid class hierarchies/relationships. Some pseudocode illustrating the concept: # declaration. the 'dispatch function' we pass can build an arbitrary value out of the args it receives. defmulti build_cell lambda { |args| args[1].tag } # if the value the dispach function generates equals :submit, this body is gonna be called defmethod build_cell :submit do # implementation... end # more defmethods for :switch, :check, :edit etc build_cell(row, cell) # our lambda gets invoked, which re-passes the arguments it receives to the corresponding implementation Multimethods doesn't seem to be have been adopted at all in the Ruby community. A quick googling reveals a couple implementations though. In the Lisp world they are first-class, even though they aren't used all the time. http://clojure.org/multimethods http://clojure.org/multimethods might be worth a read.
roestava 14y ago
Ruby is so unique. Back when I was also surprised by Ruby's reflection and metaprogramming features. So much easier than it was in the languages I had used before. Nowadays I see people wanting to change Ruby to be more like those other languages. Needless to say, Ruby is best when all of Ruby is available. Putting restrictions on it wouldn't begin to turn it into what some people want. On the server-side, people can use all kinds of languages, with no restrictions on choices. That's why these languages can flourish. On the client-side we are more restricted. The industry gets to dictate what goes on the client-side more. Sigh. I'm trying to use Dart lately which compiles to Javascript. While it can seem a bit like Ruby in OO and dynamic typing, it's a world of difference in other regards. Languages like Ruby that can evolve while breaking backward compatibility have more chance to be useful out of the box.
robbles 14y ago
Can you imagine how much more pleasant participating in HN, GitHub, etc. would be if all programmers were this pleasant and open-minded? Regardless of the particular merits of the technique he talks about here, I think we can all learn a lesson from this guy about appreciating the work and ideas of others.
- gruseom 14y ago
  I couldn't agree more. The ability to appreciate others' work is a sign of intelligence.
reyan 14y ago
I would not go for polymorphism (specially with all that meta-programming) with this simple conditional.
jakejake 14y ago
I liked the article because I feel like I'm at the stage of my career that I need to be a mentor. But at the same time I don't feel like I know enough about everything. Here is someone writing books and programming iOS apps who can still have his mind blown by, what seems to me, some fundamental concepts. I think there are a lot of people out there doing real work - who can benefit from a little push. that makes me feel that a) I have things I can teach and b) that its ok to not know everything - even what others may think is "basic" stuff.
10char 14y ago
I was confused as to how a lot of folks took the post to mean I didn't know what polymorphism was...and then I experienced the whole Arrested Development-esque "I've made a huge mistake," probably a little too late. A lot of folks drew attention to "You can reuse code with inheritance. Absolutely crazy. Brilliant." and took as equivalent to me saying "It's absolutely crazy and brilliant you can reuse code with inheritance." I get that both in- and especially out-of-context that's how it comes off, but it's not what I meant. It was sloppy and I should've been more careful in writing. I meant it to come off more like "...everything is dynamic and decided at run-time, plus you get a great plugin architecture if you do some polymorphic tricks with these RowType objects. And all of these benefits came from just one simple refactoring. The power of small refactors is absolutely crazy. Brilliant." (I've added this as an addendum to the post) Like the point of the whole thing wasn't about the refactor itself and if/how the code "rocked my world", but the fact that a refactor could have a seismic effect on the future of a project and the direction it takes. The addition of many more row "types" wasn't even in the orbit of my thinking without the refactor, and it really helped shape where the project went: there were just those 4 branches in the `if`/`else` tree in this post, but there are now 20-odd default row configurations. Hope that helps some folks to add more context
tylerc230 14y ago
Sounds like the open/closed principle of SOLID development. "Objects should be open to extension but closed to modification". This basically means that users of your code shouldn't have to modify your classes to add functionality. It should be done through inheriance and adding new types. http://en.wikipedia.org/wiki/Open/closed_principle http://en.wikipedia.org/wiki/Open/closed_principle

320 points by 10char 14y ago

herge 14y ago

The classic "Replace Conditional with Polymorphism" design pattern, which took me way to long to realize that it also applies to Python.

ghc 14y ago
Indeed it does. I have a large code base at work that uses this pattern for Django models with a bit of metaprogramming to create a template for types of input sensors. It's amazing what this kind of refactor can do when you need to turn a long conditional into something more generally extensible. New sensor types with custom column configurations take an average of 5 lines of code now, instead of about 30 before.
jonathanberger 14y ago
Yes, it's cataloged in Martin Fowler's excellent book Refactoring, Improving the Design of Existing Code in Chapter 9. I'd recommend the book strongly.
- herge 14y ago
  Link for the lazy: http://www.refactoring.com/catalog/ http://www.refactoring.com/catalog/
deltasquared 14y ago
Yup. I remember that particular realization. Now switch statements in OO languages just look broken to me. Thanks Python!
Guillaume86 14y ago
I like this kind of refactoring, make you feel like: http://thejoysofcode.com/post/36047317385/when-i-replace-200-lines-of-my-co-workers-code-with-10 http://thejoysofcode.com/post/36047317385/when-i-replace-200...

campnic 14y ago

I like Martin Fowler's Refactoring. This is called 'Replace conditional with subtype' explained here: http://www.refactoring.com/catalog/replaceConditionalWithPolymorphism.html http://www.refactoring.com/catalog/replaceConditionalWithPol... The book is a good read and a fantastic reference.

rasur 14y ago

IIRC, Fabio works at the Swiss Ruby shop Simplificator, who are all very nice chaps indeed! Glad he helped you along. EDIT: I just checked, yes he does (well, unless we're talking about another Fabio of course, which is possible...)

lincolnq 14y ago

I don't love this refactor. I especially don't like how it generates names from other names -- e.g., given 'switch'/'submit', appends '_row', and then converts it to CamelCase so you get 'SwitchRow'/'SubmitRow', then instantiates the class named that. Here's why: When I am maintaining somebody else's code, even if I guess that this is going on, I rarely trust these kinds of name-generation tricks. When I need to change some code, I'm going to grep the codebase for 'SwitchRow' and add a new line in whatever conditional to instantiate my new class. When grep can't find any instances of SwitchRow's constructor being called, I become suspicious and it costs me more time, because I am making sure I properly understand what is actually going on.

rosser 14y ago
...I rarely trust these kinds of name-generation tricks. So you're probably not a big fan of Rails, then?
- hnewser1 14y ago
  and what does `trust` mean? You don't trust that correct object won't be given back to you?
  - andreasvc 14y ago
    I suppose more like trust whether the resulting code will always be correct.
- diminish 14y ago
  what about using a multiline YAML text to be loaded with automatic generation of objects from class name strings? (reminder!)
- potatolicious 14y ago
  I'm actually not a huge fan of implicit name-generation tricks, but I'm also not entirely opposed to them. The trick here I think is consistency - is name-generated type dispatch the standard across the entirety of the codebase? If it is, go wild. If it's not then I'd be much more wary of it. Consistency and building correct expectations for future maintainers is pretty important. Doing a smart trick in one place but failing to do it in other places where it makes sense IMO tends to create a bigger mess than doing it the dumb-but-safe way everywhere.

laureny 14y ago

tl;dr: polymorphism is usually better than if/else.

bfwi 14y ago
I like this explanation: http://stackoverflow.com/questions/234458/do-polymorphism-or-conditionals-promote-better-design http://stackoverflow.com/questions/234458/do-polymorphism-or...

mquander 14y ago

I don't agree with this change. Based on the code, it looks like there should be an enumeration of cell_types somewhere, a button should have a cell_type, and then there's a map from cell_type to construction_fn (or a polymorphic set of classes to serve as the map.) Metaprogramming is something you do when you can't do it easily in the usual way. If a dispatch table or inheritance takes care of something, use the existing language features instead of building your own. (EDIT: On examination, there already is a RowType, and the pull request is a lot closer to my suggestion than I first thought, so mostly ignore this.)

jtchang 14y ago

Refactors like this are super cool and often needed but I sometimes have to think why we don't store the easier to read code as well. I know we have git and we can always do a diff to see what changed. What I am talking about is a more holistic view. Being able to search a code base for SwitchRow and then somehow the code comes up where it was refactored into this dynamic programming style. Git can do this but only if you are actively looking for it. I wish you could do a metacomment on the refactor without cluttering up the code.

shocks 14y ago
Because if you understand the new code you don't need the "easier to read" code. Constraining yourself to only writing code a beginner can understand is stupid. Maintaining good code and "easier to read for a beginner" code in tandem is a waste of man hours.

stickydink 14y ago

The article is very humble, but in my view, seems to go a little too far. "You can reuse code with inheritance. Absolutely crazy. Brilliant. All of this blew me away." Honestly?

revelation 14y ago
Thats a very critical point that you just have to understand, to "get". Many people know and learn about object oriented design, but when tasked to actually write a program based on these principles they will often revert to code where they make distinctions based on the type of what they have been handed. TLDR: dynamic_cast is bad design
- mylittlepony 14y ago
  I guess he's gonna come back with another article when he realizes composition > inheritance.
  - alttab 14y ago
    You'll need to wait like 5 years if he was anything like me
  - deleted 14y ago
    [deleted]
- to3m 14y ago
  Using static conditions rather than polymorphism seems to just be a common thing that people tend to do, regardless of skill level or quantity of experience. It's not uncommon for people just not to think of using polymorphism, if the objects in question wouldn't really model any particular kind of object as such (they're just there to provide a table of function pointers). This says nothing about the quality of their code, or the general OOness of their designs, or the number of digits in their IQ. It's a bit of a shame, because I've found code to be reliably improved by using objects as simple tables of function pointers. The same is less often true for using objects as models of things.

juddlyon 14y ago

Evocative writing for a technical post "serpentine" if/else, "rickety door" - I know nothing about iOS and enjoyed reading it.

guylhem 14y ago

On the first glance it seems great, but much more complex to understand and debug. The if tree was certainly not pretty, but straightforward - it did its thing. It was something that could be given to an average developer to work on and improve. The refactored code will require someone that's not just an average developer - if only because you need to read the article to understand it if you're not familiar with javascript, while the initial code needed only a glance to understand what it did. IMHO, that's just complexity for complexity sake - unless you have plans to take advantage of the new flexibility it provides. (and if you have to find, hire and pay that 'better than average' person, it might not be such a good thing.)

hnewser1 14y ago
This is not written in javascript, it is ruby. And I think your developer is way below average if he can't understand the pattern used in the refactored code.
- dubcanada 14y ago
  If you have a ruby programmer who can't understand this, fire them. And get a new one. This type of stuff is all over ruby code.

iamdave 14y ago

I'm not a developer, but I read this article anyway. All of this blew me away. Like this "@mordaroso" fellow strolled on up, cocked his knee skyward, and jettisoned his leg into the rickety wooden door that previously sheltered my mind. That was just great writing.

mikeklaas 14y ago
I have to disagree. "Jettison" does not fit the simile he was going for, at all.
- alecdbrooks 14y ago
  It's not great, but it could be good. Replace "jettison" and cut the cliched "All of this blew me away" and the resulting paragraph is descriptive and fairly tight.
- deleted 14y ago
  [deleted]
drakeandrews 14y ago
Jettisoning ones leg into a door sounds painful. Unless one has a prosthetic, then I must wonder why one is throwing it at a door. Do they not need it to stand?
cpeterso 14y ago
I disagree. I had to parse the sentence three times to make all the pieces fit. :\

wcoenen 14y ago

This google tech talk is basically about the same thing, eliminating conditionals with polymorphism: https://www.youtube.com/watch?feature=player_embedded&v=4F72VULWFvc https://www.youtube.com/watch?feature=player_embedded&v=...

greenyoda 14y ago

It's scary to think that this guy who was just shocked to discover a very basic concept of object oriented programming (polymorphism) has written his own book on iOS programming: http://clayallsopp.com/posts/writing-a-programming-book http://clayallsopp.com/posts/writing-a-programming-book

AlexeyBrin 14y ago
How many books did you wrote ?
- mylittlepony 14y ago
  How is that relevant?
  - AlexeyBrin 14y ago
    Just pointing that his little ad hominem attack has no place on HN.
    - mylittlepony 14y ago
      It would be an ad hominem if the book was about gardening but it's not, it's about programming.
- adamnemecek 14y ago
  Ah, the good ol' ad hominem.
- icebraining 14y ago
  That's not an Ad Hominem.
  - adamnemecek 14y ago
    I assume you are responding to me. Yes, yes it is. From Wiki "Argumentum ad hominem, is an argument made personally against an opponent instead of against their argument." AlexeyBrin is implying that because greenyoda may not have written any programming books, that his argument is less valid. AlexeyBrin's argument is not refuting greenyoda's central point, that it's somewhat strange that the blog post author has written a programming book about Ruby, despite not being aware of what some argue are the language's commonly used features.

svachalek 14y ago

In terms of Gang of Four patterns, I believe this would be Strategy + Factory Method. The book is a little antiquated (1994) but it or a better successor should be required reading for any programmer.

chris_wot 14y ago
I don't believe that its terribly antiquated. Pretty much everything bases itself off of this book. Edit: drafted iPad - fixed spelling. Trilby - does anyone even use that word anymore?!?
- martinced 14y ago
  "I don't believe that it's the trilby antiquated. Pretty much everything bases itself off of this book." Hardly. The GoF book is sweet if in this day and age you're still doing Java/C# but if you're programming in a language like, say, Clojure you can throw about 75% of the GoF book into the trash. pg said to use a Lisp to beat the average and that's what I'm doing. I'm certainly not accepting the status-quo and considering that, say, the "visitor pattern" is a godsend. Most of these "patterns" only make sense in a non-functional and OO language and are only needed because of missing languages concepts (like the lack of high-order function and/or the lack of real macros).
  - chris_wot 14y ago
    I'll be honest - I'm not terribly familiar with functional programming languages.
  - cpeterso 14y ago
    Design patterns are, admittedly, workarounds for language deficiencies, but the book is still recent because far more people program in Java and C# than Clojure.
  - svachalek 14y ago
    I don't think Clojure is really object-oriented, and therefore wouldn't recommend the use of object-oriented design patterns at all. But as the dominant paradigm of our age, I think it's worth understanding OOP even if the final decision is to reject it.

Xcelerate 14y ago

Reading the comments in this thread is very interesting to me, because each programmer seems certain of the superiority of his own programming style. We have some comments that disparage the change, saying that it obfuscates the code and makes it more difficult to understand. We have others saying that this change is a basic technique and it's "shocking" that the author wrote a book without this simple knowledge. What I take from this is yet another situation of people arguing about something highly subjective that is closely tied to their identity. Good programming is not an objective concept. People respond differently to different programming techniques and what works better for one person may greatly confuse another.

AlexeyBrin 14y ago
+1 for Good programming is not an objective concept.
- nikster 14y ago
  I dont agree with that. There is a confusion about programming though, an idea that it is a purely technical discipline, like laying bricks when in reality its a design discipline, like designing a building. If you wanted to build a house you wouldnt just hire a couple of bricklayers and tell them "do it". Yet thats what happens in programming...
- dakimov 14y ago
  == Programming sucks nowadays. There is not even a consensus on what is good in programming! Take a look at other professional cultures: people work extremely hard to develop the skills that everybody else in their profession have. What does a software developer do? Whatever they like on their own, there is no professional culture whatsoever: "Don't give a crap about the humankind's experience, I'll invent everything on my own the way I like". This is actually unbelievable how different programming is in this aspect from other disciplines. I don't even mentor Junior Developers because of that, as I can't refer to anything authoritative and say "Do it like this" because there is always a bunch of folks that do it the opposite way. There is almost nothing to rely on.

sbuccini 14y ago

To be honest, I'm very disappointed with the comments here. Clay is obviously a very talented programmer, and just like most talented programmers, he didn't start out that way. This article isn't about how he learned OOP, it's about how someone opened his eyes to how code can be elegant instead of simply just functional. This article hits home with me especially. I just learned Python last semester in my intro to CS class. I had an eerily similar moment when I learned how to use generators. "WOW! You can simplify this huge block of code into one line! It's..beautiful." It's a really special moment, and I'm glad Clay shared his.

leriksen 14y ago

Those who know this technique either discover it themselves, through "reading, learning, doing", or are shown,like this guy was. None of us was born knowing this. Clay has blogged about his experience of discovering this - that in itself is what makes this a great read for me. I wouldn't be surprised if every single tech book author, at some point after their book has been published, has learned something and thought "I wish I knew that when I wrote my book !!" By their very nature books document the authors understanding at a point in time. Would polymorphism have helped Clay's book ? Probably. Does not knowing it ruin the book ? Probably not. If we waited 'til we knew everything before we wrote a book about anything, nothing would ever get written.

AlexeyBrin 14y ago
Writing a book is also a great way to discover new things about your subject.
- nirvdrum 14y ago
  While true, when I purchase a book I'm trusting that the author is an expert in the topic. Maybe my expectations are too high.
  - chris_wot 14y ago
    You may never know - if it teaches you some things and makes you more productive, then what of it?
    - nirvdrum 14y ago
      My concern is the inverse of that: recommending something you either don't fully understand or haven't used long enough to speak to either its pros or cons. Being in a book usually is an indicator that something is a best practice or how the way things should be done, ostensibly being put forth by an expert. Perhaps we shouldn't hold such content to such a standard.

chaz 14y ago

Critiquing these particular lines of code or the pull request is unimportant. My takeaway was that it's hugely valuable to simply make your code available and have other people look at it. Not only can it improve your own code, but you can personally learn something new from other people. You can be sure Now I'm feeling a bit guilty for not having open sourced any of the code I've ever written, and I'm wondering what kinds of lessons I could have learned earlier on.

rckrd 14y ago

I would like to point out that he was able to describe his original implementation in a few sentences. This is important.

jules 14y ago

Novice programmer: Uses simple if/else. Adept programmer: Discovers polymorphism. It's great, uses it for everything. Master programmer: Realizes that in many cases, it makes the code less clear and more verbose, reverts back to if/else until the flexibility is actually needed. Same story repeats itself many times on different fronts: metaprogramming, recursion, exceptions, macros, etc.

dougk16 14y ago
One more step: Master programmer realizes that they're still a Novice programmer - repeats the cycle.

rachelbythebay 14y ago

I tried to picture the solution before scrolling down past the initial block with the nested ifs. It got me thinking about "switch()", as seen in C, C++, and elsewhere. That in turn got me thinking about using an enum to pick up on whatever it is he's trying to do here, and jump to the right place accordingly. Generically, I had something like this in my head: switch (row_type) { case submittable: ...; break; case checkable: ...; break; ... } Then I started realizing that using a switch means you have isolated things such that they can only be one of the n types. It won't ever be submittable AND checkable, for instance. I assume that's what is desired here: mutual exclusivity for all of these possibilities. Looking back at the original code, I see that by using the nested if/else construct, it effectively gives priority to them in the order in which they are encountered. A row with a submit_button is going to fire off make_submit_cell() whether or not it has checkable set. It trumps the possible call to make_check_cell, in other words. These two approaches solve two different problems. Without knowing more, it's hard to say whether the "priority" approach makes sense here. If I had to guess, based on the refactoring which turned it into a series of distinct classes, that sounds like it should have been "pick 1 of n" (one-hot code) all along. If that's so, the if block wasn't just ugly, but it was actually covering up some real problems. It was just asking for trouble in terms of bugs down the road since it allowed nonsensical settings. But that's just a guess. I could be wrong.

potatolicious 14y ago
Indeed, the original code was, IMO, incorrect. It looks like the purpose is a pretty common one in iOS - you have a table view populated by many rows, each of which can be a different class that renders and behaves differently. It's a common need in iOS apps. What he really needed was an enum, not flags. The choice of class is mutually exclusive and flags are horrible for tracking this, and are great for hiding bugs - one erroneous flag flip and suddenly your code is walking down a completely different path that's non-obvious on first debug. Using a bunch of bools to approximate state is error-prone and IMO just bad architecture.

jQueryIsAwesome 14y ago

I do something a little bit similar with DOM interaction in JS, for example button ".btn_pdf_docs" shows all ".pdf_docs" elements. I consider the code to implement this more readable than a bunch of events binding.

jamieb 14y ago

This person found something that they didn't know they didn't know and were so happy they blogged about it. Regardless of what I may personally think about the merits of the code before or after, it makes me happy that there is another human being who is thrilled by learning something new about programming.

deleted 14y ago

[deleted]

davidrudder 14y ago

I like it when developers praise each other.

vemv 14y ago

Fabio's approach does the job, but it fails at genericity - its usefulness is limited to the scope of the Formotion project. Which is unfortunate because something actually simple is going on - a mapping of keywords to functions. What you really want is multimethods - they provide all the goodness of classic polymorphism, without forcing you to model intrincate, rigid class hierarchies/relationships. Some pseudocode illustrating the concept: # declaration. the 'dispatch function' we pass can build an arbitrary value out of the args it receives. defmulti build_cell lambda { |args| args[1].tag } # if the value the dispach function generates equals :submit, this body is gonna be called defmethod build_cell :submit do # implementation... end # more defmethods for :switch, :check, :edit etc build_cell(row, cell) # our lambda gets invoked, which re-passes the arguments it receives to the corresponding implementation Multimethods doesn't seem to be have been adopted at all in the Ruby community. A quick googling reveals a couple implementations though. In the Lisp world they are first-class, even though they aren't used all the time. http://clojure.org/multimethods http://clojure.org/multimethods might be worth a read.

roestava 14y ago

Ruby is so unique. Back when I was also surprised by Ruby's reflection and metaprogramming features. So much easier than it was in the languages I had used before. Nowadays I see people wanting to change Ruby to be more like those other languages. Needless to say, Ruby is best when all of Ruby is available. Putting restrictions on it wouldn't begin to turn it into what some people want. On the server-side, people can use all kinds of languages, with no restrictions on choices. That's why these languages can flourish. On the client-side we are more restricted. The industry gets to dictate what goes on the client-side more. Sigh. I'm trying to use Dart lately which compiles to Javascript. While it can seem a bit like Ruby in OO and dynamic typing, it's a world of difference in other regards. Languages like Ruby that can evolve while breaking backward compatibility have more chance to be useful out of the box.

robbles 14y ago

Can you imagine how much more pleasant participating in HN, GitHub, etc. would be if all programmers were this pleasant and open-minded? Regardless of the particular merits of the technique he talks about here, I think we can all learn a lesson from this guy about appreciating the work and ideas of others.

gruseom 14y ago
I couldn't agree more. The ability to appreciate others' work is a sign of intelligence.

reyan 14y ago

I would not go for polymorphism (specially with all that meta-programming) with this simple conditional.

jakejake 14y ago

I liked the article because I feel like I'm at the stage of my career that I need to be a mentor. But at the same time I don't feel like I know enough about everything. Here is someone writing books and programming iOS apps who can still have his mind blown by, what seems to me, some fundamental concepts. I think there are a lot of people out there doing real work - who can benefit from a little push. that makes me feel that a) I have things I can teach and b) that its ok to not know everything - even what others may think is "basic" stuff.

10char 14y ago

I was confused as to how a lot of folks took the post to mean I didn't know what polymorphism was...and then I experienced the whole Arrested Development-esque "I've made a huge mistake," probably a little too late. A lot of folks drew attention to "You can reuse code with inheritance. Absolutely crazy. Brilliant." and took as equivalent to me saying "It's absolutely crazy and brilliant you can reuse code with inheritance." I get that both in- and especially out-of-context that's how it comes off, but it's not what I meant. It was sloppy and I should've been more careful in writing. I meant it to come off more like "...everything is dynamic and decided at run-time, plus you get a great plugin architecture if you do some polymorphic tricks with these RowType objects. And all of these benefits came from just one simple refactoring. The power of small refactors is absolutely crazy. Brilliant." (I've added this as an addendum to the post) Like the point of the whole thing wasn't about the refactor itself and if/how the code "rocked my world", but the fact that a refactor could have a seismic effect on the future of a project and the direction it takes. The addition of many more row "types" wasn't even in the orbit of my thinking without the refactor, and it really helped shape where the project went: there were just those 4 branches in the `if`/`else` tree in this post, but there are now 20-odd default row configurations. Hope that helps some folks to add more context

tylerc230 14y ago

Sounds like the open/closed principle of SOLID development. "Objects should be open to extension but closed to modification". This basically means that users of your code shouldn't have to modify your classes to add functionality. It should be done through inheriance and adding new types. http://en.wikipedia.org/wiki/Open/closed_principle http://en.wikipedia.org/wiki/Open/closed_principle