JILLIAN FOLEY • RAILSCONF 2015
SO YOU WANT
•Software engineer at Contactually •Learned coding on-the-job •Pretty much only worked in legacy code
SO WHAT IS REFACTORING?
“Refactoring is a disciplined technique for restructuring an existing body of code, altering its
internal structure without changing its external behavior."
THE IDEA IS
• No change to your users • Improvement for
BUT … WHY?
“Refactoring? No way. We need new features!”
• Maintainability • Scalability • Flexibility • Spring cleaning
HOW WILL I KNOW WHEN IT’S TIME TO REFACTOR?
OVERWHELMED BY BUGS
DECREASED PERFORMANCE N00B SAYS WTF
TAKE A LOOK
WINTER IS COMING
TESTS ARE HARD TO WRITE
BLOCKING NEW PROGRESS
DO IT YESTERDAY!
SO HOW DO YOU DO IT?
P R E PA R E
MISE EN PLACE, FOR CODE
• Beef up your test coverage
• Review functionality x 100 • Brush up on code style
guides and preferences
MARK UP SMELLY CODE
• Read through and comment anything that looks weird, bad, wrong, or confusing
• Consider copying dependencies temporarily
• Rename variables and methods
LOOK FOR CODE MESSES
• What looks redundant? • What looks too complex? • What parts of the code took more
than 1-2 reads to understand? • Do you see any methods that
look bloated? • Does any of it violate code style
GIVE YOUR CODE A MAKEOVER
• Focus on one method at a time • DRY it up • Pick better names • Fix style issues • Simplify conditionals • Break down complex methods • Move code where it truly belongs
A VERY BRIEF CASE STUDY
email = params[:signup].delete(:email).to_s.gsub(/\s/, '').downcase[0..100]
unless u = User.find_by_email(email) if a = Account.email.find_by_username(email)
u=a end end
if u if email != "email@example.com" flash[:notice] = "Looks like you already have an account." redirect_to new_user_session_path(:exists => true) return true
else sign_in(u) redirect_to signup_accounts_path return true
# Does this assignment need to be so long? Why is the param key :signup ? email = params[:signup].delete(:email).to_s.gsub(/\s/, '').downcase[0..100]
# This nested unless/if makes my brain hurt. unless u = User.find_by_email(email)
if a = Account.email.find_by_username(email) # Is there some way to consolidate these two u assignments? u = a # wtf are these single-letter variables
# Ew another nested if if u
if email != "firstname.lastname@example.org" flash[:notice] = "Looks like you already have an account." redirect_to new_user_session_path(:exists => true) return true
# Using the negative in the first if makes this 'else' confusing else
sign_in(u) redirect_to signup_accounts_path return true end end
email = params[:user][:email]
# Try to find an existing user by email or account existing_user = User.find_by_email(email) ||
# Push demo user through to demo signup if existing_user && email == 'email@example.com'
sign_in(existing_user) redirect_to signup_accounts_path return true end
if existing_user flash[:notice] = "Looks like you already have an account with
Contactually." redirect_to new_user_session_path(:exists => true) return true
• Test frequently, both manually and automated. • Don’t change tests while refactoring! • Commit frequently. • Use tools like Rubocop or Reek for help when you’re
overwhelmed or stuck • Consider pausing after a logical group of changes to
do more testing, code review, and deploy . . .
SLOW AND STEADY WINS THE RACE?
OMG I WANT ALL THE CHANGES!!!11
THINGS TO REMEMBER
• Refactoring isn’t scary • It’s a good way to learn new design patterns • It’s a good way to learn your codebase • Tests can be refactored too! • Go back and refactor old projects for practice
GO FORTH AND REFACTOR!
J I L L I A N @ C O N TA C T U A L LY. C O M @J3FOLEY
APPENDIX: FURTHER READING
• Martin Fowler's site to accompany his Refactoring book • The Ruby version of above Refactoring book, by Jay
Fields, Shane Harvie, and Martin Fowler • Ginny Hendry's notes from the Ruby version of the
Refactoring book • Codecademy Ruby refactoring exercise