TE
TechEcho
Home24h TopNewestBestAskShowJobs
GitHubTwitter
Home

TechEcho

A tech news platform built with Next.js, providing global tech news and discussions.

GitHubTwitter

Home

HomeNewestBestAskShowJobs

Resources

HackerNews APIOriginal HackerNewsNext.js

© 2025 TechEcho. All rights reserved.

How A Pull Request Rocked My World

320 pointsby 10charover 12 years ago

31 comments

Xcelerateover 12 years 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.<p>We have some comments that disparage the change, saying that it obfuscates the code and makes it more difficult to understand.<p>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.<p>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.
评论 #5157649 未加载
评论 #5158864 未加载
评论 #5159676 未加载
评论 #5158275 未加载
评论 #5157660 未加载
评论 #5158976 未加载
评论 #5157583 未加载
评论 #5159246 未加载
评论 #5158238 未加载
lincolnqover 12 years 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.<p>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.
评论 #5157414 未加载
评论 #5157521 未加载
评论 #5157564 未加载
评论 #5157428 未加载
julesover 12 years ago
Novice programmer: Uses simple if/else.<p>Adept programmer: Discovers polymorphism. It's great, uses it for everything.<p>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.<p>Same story repeats itself many times on different fronts: metaprogramming, recursion, exceptions, macros, etc.
评论 #5158667 未加载
rachelbythebayover 12 years 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.<p>Generically, I had something like this in my head:<p><pre><code> switch (row_type) { case submittable: ...; break; case checkable: ...; break; ... } </code></pre> 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.<p>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.<p>These two approaches solve two different problems. Without knowing more, it's hard to say whether the "priority" approach makes sense here.<p>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.<p>But that's just a guess. I could be wrong.
评论 #5157972 未加载
sbucciniover 12 years ago
To be honest, I'm very disappointed with the comments here.<p>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 <i>elegant</i> instead of simply just functional.<p>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.
10charover 12 years 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.<p>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.<p>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)<p>Like the point of the whole thing wasn't about the refactor itself and if/how <i>the code</i> "rocked my world", but the fact that <i>a refactor</i> could have a <i>seismic effect</i> on the future of a project and the direction it takes.<p>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.<p>Hope that helps some folks to add more context
mquanderover 12 years 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.)<p>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.<p>(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.)
guylhemover 12 years ago
On the first glance it seems great, but much more complex to understand and debug.<p>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.<p>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.<p>IMHO, that's just complexity for complexity sake - unless you have plans to take advantage of the new flexibility it provides.<p>(and if you have to find, hire and pay that 'better than average' person, it might not be such a good thing.)
评论 #5157532 未加载
leriksenover 12 years 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.
评论 #5157672 未加载
评论 #5157628 未加载
robblesover 12 years ago
Can you imagine how much more pleasant participating in HN, GitHub, etc. would be if all programmers were this pleasant and open-minded?<p>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.
评论 #5158912 未加载
stickydinkover 12 years ago
The article is very humble, but in my view, seems to go a little too far.<p>"You can reuse code with inheritance. Absolutely crazy. Brilliant.<p>All of this blew me away."<p>Honestly?
评论 #5157419 未加载
评论 #5157385 未加载
评论 #5158390 未加载
评论 #5157665 未加载
评论 #5157445 未加载
评论 #5157465 未加载
chazover 12 years 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<p>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.
hergeover 12 years ago
The classic "Replace Conditional with Polymorphism" design pattern, which took me way to long to realize that it also applies to Python.
评论 #5157234 未加载
评论 #5157752 未加载
评论 #5157688 未加载
评论 #5157874 未加载
wcoenenover 12 years ago
This google tech talk is basically about the same thing, eliminating conditionals with polymorphism: <a href="https://www.youtube.com/watch?feature=player_embedded&#38;v=4F72VULWFvc" rel="nofollow">https://www.youtube.com/watch?feature=player_embedded&#38;v=...</a>
jamiebover 12 years 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.
rckrdover 12 years ago
I would like to point out that he was able to describe his original implementation in a few sentences. This is important.
laurenyover 12 years ago
tl;dr: polymorphism is usually better than if/else.
评论 #5157290 未加载
jtchangover 12 years 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.<p>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.<p>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.
评论 #5157990 未加载
vemvover 12 years 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.<p>What you really want is multimethods - they provide all the goodness of classic polymorphism, without forcing you to model intrincate, rigid class hierarchies/relationships.<p>Some pseudocode illustrating the concept:<p><pre><code> # 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 </code></pre> Multimethods doesn't seem to be have been adopted at all in the Ruby community. A quick googling reveals a couple implementations though.<p>In the Lisp world they are first-class, even though they aren't used all the time. <a href="http://clojure.org/multimethods" rel="nofollow">http://clojure.org/multimethods</a> might be worth a read.
roestavaover 12 years 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.<p>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.<p>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.<p>Sigh.<p>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.
greenyodaover 12 years 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:<p><a href="http://clayallsopp.com/posts/writing-a-programming-book" rel="nofollow">http://clayallsopp.com/posts/writing-a-programming-book</a>
评论 #5157492 未加载
评论 #5157469 未加载
评论 #5157490 未加载
rasurover 12 years ago
IIRC, Fabio works at the Swiss Ruby shop Simplificator, who are all very nice chaps indeed! Glad he helped you along.<p>EDIT: I just checked, yes he does (well, unless we're talking about another Fabio of course, which is possible...)
jakejakeover 12 years 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.<p>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.
tylerc230over 12 years 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. <a href="http://en.wikipedia.org/wiki/Open/closed_principle" rel="nofollow">http://en.wikipedia.org/wiki/Open/closed_principle</a>
campnicover 12 years ago
I like Martin Fowler's Refactoring. This is called 'Replace conditional with subtype' explained here: <a href="http://www.refactoring.com/catalog/replaceConditionalWithPolymorphism.html" rel="nofollow">http://www.refactoring.com/catalog/replaceConditionalWithPol...</a><p>The book is a good read and a fantastic reference.
svachalekover 12 years 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.
评论 #5157554 未加载
juddlyonover 12 years ago
Evocative writing for a technical post "serpentine" if/else, "rickety door" - I know nothing about iOS and enjoyed reading it.
davidrudderover 12 years ago
I like it when developers praise each other.
reyanover 12 years ago
I would not go for polymorphism (specially with all that meta-programming) with this simple conditional.
jQueryIsAwesomeover 12 years 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.
iamdaveover 12 years ago
I'm not a developer, but I read this article anyway.<p><i>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.</i><p>That was just great writing.
评论 #5157412 未加载
评论 #5157720 未加载
评论 #5157843 未加载