A couple of weeks ago, I was talking to a friend about the impeding launch of Discourse. She asked me if I was worried that people would judge me for the code I'd written (although to be honest, you can't tell who wrote what since we reset our commit history when we launched).<p>The truth is releasing your code is a very scary thing to do. I knew some parts were good, some parts less so.<p>Posts like this (and their pull requests) have floored me. Grant did a fantastic job refactoring a large chunk of technical debt we'd acquired, and then took the time to write up why he did it in detail.<p>I really hope this trend of improving something and writing how you did it catches on. I want to see it in other people's code bases too!
I really enjoyed reading this!<p>When Discourse was originally announced, I read this file (quite randomly), and was distressed at the complexity and length (even though I totally understand time constraints and the need to 'get it to work').<p>I had never heard of the Law of Demeter (<a href="http://en.wikipedia.org/wiki/Law_of_Demeter" rel="nofollow">http://en.wikipedia.org/wiki/Law_of_Demeter</a>), nor had I heard of Sandi Metz's rules for Rails development (<a href="http://gist.io/4567190" rel="nofollow">http://gist.io/4567190</a>), both of which put in concrete terms some of the lessons I've learned when maintaining Rails applications.<p>I'd love to see more of these!<p>Edit: After reading over the pull request, I admit I'm a little confused why the response code was refactored into 'perform_show_response' - this seems like the direct responsibility of this controller method (and isn't common or otherwise misplaced). Is there a reason why you moved it out?
This post is great, I'd love to see more blog posts about refactorings of open source code on Hacker News.<p>It's good to be reminded of refactoring techniques, and to be reminded of what neatly refactored complex code looks like.<p>I think being able to refactor code, and make it look that good is a hallmark of a great programmer.
Good overall, but is there justification for having consider_user_for_promotion called through a before_filter? It doesn't filter anything or redirect the request. It also doesn't retrieve instance variables for the request to process. You could theoretically put the whole method being refactored into a before_filter, but that's just a shell game.<p>I agree with making consider_user_for_promotion a standalone function. I like that because it is reusable, and self-comments the code.