Aloha Ruby Conf 2012 Refactoring from Good to Great by Ben Orenstein

Most developers know enough about refactoring to write code that's pretty good. They create short methods, and classes with one responsibility. They're also familiar with a good handful of refactorings, and the code smells that motivate them.
This talk is about the next level of knowledge: the things advanced developers know that let them turn good code into great. Code that's easy to read and a breeze to change.
These topics will be covered solely by LIVE CODING; no slides. We'll boldly refactor right on stage, and pray the tests stay green. You might even learn some vim tricks as well as an expert user shows you his workflow.
Topics include:
The Open-Closed Principle The types of coupling, and their dangers Why composition is so damn great A powerful refactoring that Kent Beck refers to as "deep deep magic" How to destroy conditionals with a NullObject The beauty of the Decorator pattern Testing smells, including Mystery Guest and stubbing the system under test The stuff from the last halves of Refactoring and Clean Code that you never quite got to.
Help us caption & translate this video!
amara.org/v/FGgU/

[@sanjitsingh8852]

Watching this after 9 years. This is a great talk.

[@Andriak2]

This is my favorite talk to listen to in the background while i program.

[@muntoonxt]

Wait. Do you listen to the same talk while you program?

[@MrPDTaylor]

This was the best lecture I've heard in a while.

[@Truthiness231]

I have some source from just a week ago to compare with what I've done in the week since I first watched this video... went from stuff that looks OK - would be considered finished by most - to (and I'm not exaggeration here) something I was able to show my 80+ year old grandmother and she could make some sense of it (since it's pretty much just reading English at the level of granularity preached here). Amazing how far I've come after absorbing this talk, thanks very much for this. ^.^

[@r00k123123]

I'm flattered!

[@WilkerLucio]

one of the best presentations I said, very good man!

[@rimian]

Ben mentions Martin Fowler's book on refactoring at the end. FYI, his original book is written mainly for Java devs. There is a ruby version he has also co authored with Jay Fields called Refactoring: Ruby Edition.

[@baterchen]

Thanks for your great talk, I really learn a lot from it!!!

[@binarymuse007]

I personally would have loved to have seen (and would likely implement) an extraction of a Subscription class in this example. Definitely a great talk!

[@bertobertoberto3]

wow. That's an excellent talk! Kudos.

[@A07WeDoWebGood]

Thanks for the great talk!

[@itsukiuehara6292]

you are a genius mr aloha ruby man...

[@mbigras]

You rock Ben!

[@r00k123123]

Thanks!

[@r00k123123]

Thank you, bionic sir.

[@pasxizeis]

Thanks also! Great!

[@drobe121]

That was a very informative talk. I'm curious to know how you would write the test for the User class after you extracted out PaymentGateway. It seems like the only thing you could test is that the User class is calling the PaymentGateway. If that was tested, would it introduce coupling between the test and the implementation of the class? Or is it inevitable that some tests will know more about the internals of a class (e.g. that it calls a specific method on PaymentGateway)?

[@codatrainer3988]

I love the use of macro, already using vim from long time, but never used macro to do this extraction, search but could not find this macro.

[@mamayiwe]

Great talk. What vim theme is he using?

[@mtheoryx83]

VIM master, good lord

[@QuakePhil]

10:25 even vim gods waste time unfucking vim lol

[@modolief]

Github repo for this talk is here: github.com/r00k/refactoring-good-to-great

[@tooManyMidgets]

someone link me to a way i can do that macro method magic he does in ST3. I am like a total noob, but love hotkeys like this.

[@Dimitry-Nazarov]

around 13min, Ben is constructing the DateRange class using Struct with keys. Why go that route vs instantiating a simple class with an attr_reader? Is this some sort of a shortcut?

[@manojmj5479]

both have the same effect. It's just that using Struct gives you the required outcome in much fewer lines of code.

[@JOKBO1]

This talk has so much...class :D BA DUM TSS

[@r00k123123]

Not sure. Maybe! Give it a try and see how it goes. If you struggle, it's probably not a good technique for that scenario :) Seriously though, our CTO (whose opinion I respect greatly on these things) says he often doesn't follow "Tell, Don't Ask" in the view, since views are pretty much ALL asks. So maybe this technique isn't a great fit here.

[@itsukiuehara6292]

I see that you make a function called include? but uses range#cover inside it... kinda confusing but it differs person to person xD

[@andybezaire7670]

tell don't ask, Why not move this code into Order in a method called generate report? The OrderReport object seems to need to unnecessarily query the order a lot. just have the order generate the report for itself?

[@andybezaire7670]

great talk, made it all the way to the end. a nice one for people looking for philosophy AND practice

[@thatoneuser8600]

What's your solution to TDA for when the objects you're querying already have many responsibilities? Sure, following TDA makes classes highly cohesive, but surely delegating even more responsibilities would potentially make important classes violate SRP?

[@antonionalesso6484]

Here I'm again, if figured out that I have to share this, then it could've been written like so: class NullObject def method_missing(name) "no #{name}" end end However you'd still have to define deliver_personalised_email (otherwise it'd've failed) or could create something to handle it. Matt Wynne also gave this awesome talk: watch?v=CGN4RFkhH2M Uncle Bob KeyNote: watch?v=WpkDN78P884

[@sandeepkrav]

Ben Orenstein you have used open struct in this particular presentation, regarded as one of the slowest in benchmarking. (I presume with time you also not using the same unless I missed something else :) )

[@phillipproll9321]

28:05 - now delegate!

[@ozzyaararon]

Polymorphism looks a lot like "Steal second".

[@nicolasmaloeuvre]

16:50 why not delete Order class (l.28-32) and replace l.8 by : @orders.select{ |order|@date_range.include?(order.placed_at)}

[@RubyLoveIO]

nicolas maloeuvre probably because expert rubyists tend to create classes rather than allowing primitives to do the work. One reason for this is encapsulation. I show a similar principle here: JhIYw1Y7pY0

[@yahoobing]

I think it's blackboard.

[@TreesPlease42]

This guy is preaching from the OOP bible. I like OOP, and he's got a lot of good points, but he takes almost everything to its logical conclusion and that's often not correct. EG, throwing everything into one function is perfectly fine if that behavior is unique. Such a function should only be refactored when it's proven that the behavior is useful in more general cases. Similarly it's often not a good idea to cram behavior onto a class just because you're doing something with that class. You 'd end up with a confusing mess of a class. Good talk but I really hope people don't blindingly drink the OOP koolaid.

[@QuakePhil]

I thought it was as cogent and as valid as the point in the video. Refactoring is great but shipped designs change rapidly also.

[@kobac8207]

"Such a function should only be refactored when it's proven that the behavior is useful in more general cases." It's because your trigger for refactoring is reusability. Seems like his trigger is making implicit code explicit. Both triggers are valid and valuable, but it's important to be aware that there are different things people value when refactoring and that they do it for a different goal.

[@rajgurung874]

like he mentioned, none of the refactor he has showed will work on every instances. There is a pros that one need to identify if it is of worthy cause but in general (imo) well designed code is always easier to maintain.

[@adm3333]

Very strongly disagree. Readability is the most important thing, and throwing everything into one function makes it more difficult to read. He even made this point over and over. This isn't OOP specific either.

[@aantix]

You read code more than you change it. And a majority of code, does not change. Look at your git annotations - some code is very actively changed, a large majority stabilizes over 1,2,3 years. If we can't predict in advance which code is going to change, why would we prematurely optimize and make it all flexible upfront by extracting to these additional methods? So much indirection. Imagine the amount of mental overhead introduced by this in the name of "it may change in the future". E.g. Bouncing back and forth between method definitions just to see what a five-line method is trying to achieve? Deconstructing perfectly legible, serialized commands into a thousand levels of granularity may make those lines more flexible, but at the cost of code legibility. Break the lines out when it needs to be done, for DRYness, to clarify a non-obvious query, non-obvious logic. But to refactor a call of `time = Time.current` to its own method `def current_time`, is completely unnecessary.

[@camaradearthur3531]

Meanwhile in y company, with thousands classes impossible to understand because they are all tangled and code is spread accross thousands 10 lines files... I disagree with this talk. Don't create too many classes and private methods, do that when it is needed and make sure you don't need to understand underlying structures to understand the interfaces.

[@masynchin]

Create OrdersInDateRange class, and remove all date logic from Report. Without this change code isn't even a good, not saying great

[@alexsmart2612]

The problem with replacing conditionals with polymorphism is that all of that behavior needs to be put inside the class which often does not adhere to the single responsibility principle. For instance, in the Contact example, you needed to have virtual dispatch on the method Contact::deliver_personalized_email, which means that the Contact class needs to know how to deliver emails. Does your Contact class really need to know how to deliver emails? Does your Contact class need to depend on your EmailService? Would it not have been actually much neater to say something like @email_service.deliver(contact.email, email_body) inside JobSite?