Two Trains and Other Refactoring Analogies

Have you ever heard someone say, 'This code sucks. We need to rewrite it.'?

I've been there and it usually doesn't end well. We'll discuss analogies around continual improvement and how to avoid declaring technical bankruptcy.

Presented at a SoCal Python meetup: http://www.meetup.com/socalpython/events/233187442/

Transcript

1. TWO TRAINS AND OTHER REFACTORING ANALOGIES by Kevin London @kevin_london ChowNow
2. Legacy Code
3. Formula: 1. No tests 2. Hard to read. 3. Hard to change.
4. “Any fool can write code that a computer can understand. Good programmers write code that humans can understand.” Martin Fowler
5. It’s easy to create legacy code.
6. How can we prevent it?
7. 1. Add tests. 2. Refactor.
8. Refactoring: Improving design without changing behavior
9. My analogies for refactoring:
10. The Trash Pile
11. “Who will notice if I add this piece of trash?”
12. Better to clean as we go.
13. An example.
14. def get_tax(order): if order.state == 'CA': return order.subtotal * .0875 elif order.state == 'PA': return order.subtotal * .08 else: raise NotImplementedError()
15. Need to add in Texas.
16. def get_tax(order): if order.state == 'CA': return order.subtotal * .0875 elif order.state == 'PA': return order.subtotal * .08 elif order.state == 'TX': return order.subtotal * .0825 else: raise NotImplementedError()
17. def get_tax(order): if order.state == 'CA': return order.subtotal * .0875 elif order.state == 'PA': return order.subtotal * .08 else: raise NotImplementedError()
18. Add tests.
19. import pytest from taxes import get_tax, Order def test_get_tax_for_ca(): order = Order(subtotal=10, state='CA') assert get_tax(order) == 10 * .0875 def test_get_tax_for_pa(): order = Order(subtotal=10, state='PA') assert get_tax(order) == 10 * 0.08 def test_it_raises_error_for_others(): with pytest.raises(NotImplementedError): order = Order(subtotal=1, state='EQ') assert get_tax(order) == 0
20. Refactor.
21. TAXES = { 'CA': .0875, 'PA': .08, } def get_tax(order): try: return order.subtotal * TAXES[order.state] except KeyError: raise NotImplementedError()
22. ============ test session starts ================= test.py ... ============ 3 passed in 0.01 seconds ============
23. def tax_equals_percent(state, percent): subtotal = 10 order = Order(subtotal, state) return get_tax(order) == subtotal * percent . . . def test_get_tax_for_texas(): assert tax_equals_percent(‘TX', .0825) . . .
24. ============ test session starts ============ test.py ..F. ===== 1 failed, 3 passed in 0.02 seconds ====
25. TAXES = { 'CA': .0875, 'PA': .08, 'TX': .0825, } def get_tax(order): try: return order.subtotal * TAXES[order.state] except KeyError: raise NotImplementedError()
26. ============ test session starts ================= test.py .... ============ 4 passed in 0.01 seconds ============
27. Make it better over time.
28. Tube of Toothpaste
29. Easy to get toothpaste from a new tube.
30. At the end, need to roll it to get toothpaste
31. With Code: Make it easy to change.
32. “For each desired change: make the change easy (warning: this may be hard), then make the easy change.” Kent Beck
33. class User: def __init__(self, data): self.data = data def validate(self): if '@' not in self.data['email']: raise ValueError('Invalid email provided.') if len(self.data['first_name']) > 50: raise ValueError('First name too long.') if len(self.data['last_name']) > 100: raise ValueError('Last name too long.')
34. Change: Validate phone number
35. import pytest def assert_raises_error(data, expected_message): user = User(data=data) with pytest.raises(ValueError) as excinfo: user.validate() assert expected_message in str(excinfo.value) class TestValidateUser: def test_invalid_email_raises_error(self): data = { 'first_name': 'foo', 'email': 'bademail', 'last_name': 'lee', } assert_raises_error(data, 'Invalid email provided') . . .
36. =========== test session starts ======= test.py ... ======== 3 passed in 0.01 seconds=====
37. Make the change easy.
38. class User: def __init__(self, data): self.data = data def validate(self): self.validate_email() self.validate_first_name() self.validate_last_name() def validate_email(self): if '@' not in self.data['email']: raise ValueError('Invalid email provided.') def validate_first_name(self): if len(self.data['first_name']) > 50: raise ValueError('First name too long.') def validate_last_name(self): if len(self.data['last_name']) > 100: raise ValueError('Last name too long.')
39. =========== test session starts ======= test.py ... ======== 3 passed in 0.01 seconds=====
40. class User: _fields = [ 'email', 'first_name', 'last_name' ] def __init__(self, data): self.data = data def validate(self): for field in self._fields: method_name = 'validate_{}'.format(field) validate_field = getattr(self, method_name) validate_field() def validate_email(self): . . .
41. =========== test session starts ======= test.py ... ======== 3 passed in 0.01 seconds=====
42. Make the easy change.
43. class TestValidateUser: . . . def test_invalid_phone_number_raises_error(self): data = {'phone_number': 'no_phone'} user = User(data) with pytest.raises(ValueError): user.validate_phone_number()
44. =========== test session starts ======= test.py ...F == 1 failed, 3 passed in 0.01 seconds==
45. class User: _fields = [ 'email', 'first_name', 'last_name', 'phone_number', ] PHONE_LENGTH = 10 . . . def validate_phone_number(self): if len(self.data['phone_number']) != User.PHONE_LENGTH: raise ValueError('Phone number is wrong length.')
46. =========== test session starts ======= test.py .... ======== 4 passed in 0.01 seconds======
47. By refactoring, we make it easy to change the code.
48. The Two Trains
49. Need to jump.
50. The nuclear option: rewrite
51. With code: 1. Reach Parity 2. Switch 3. Improve
52. A story about a failed rewrite.
53. Let’s say we’re switching languages.
54. PHP -> Python
55. <?php if (isset($_GET['fahrenheit'])) { $fahrenheit = $_GET['fahrenheit']; } else { die('No degrees in Fahrenheit provided.'); } $celsius = ($fahrenheit - 32) * 5 / 9; echo floor($celsius);
56. 1. Add tests. 2. Refactor.
57. import requests API_URL = 'http://localhost:8888/celsius' def test_api_converts_fahrenheit(): degrees_f = 32 degrees_c = 0 path = '{}?fahrenheit={}'.format(API_URL, degrees_f) response = requests.get(path) assert int(response.content) == degrees_c def test_api_rounds_down(): degrees_f = 100 degrees_c = 37 # rounded down path = '{}?fahrenheit={}'.format(API_URL, degrees_f) response = requests.get(path) assert int(response.content) == degrees_c def test_api_returns_error_if_no_degrees(): response = requests.get(API_URL) assert 'No degrees' in response.content
58. =========== test session starts ======= test_php_api.py ... ======== 3 passed in 0.01 seconds=====
59. Rewrite the code.
60. import requests API_URL = 'http://localhost:5000/celsius' . . .
61. =========== test session starts ======= test_py_api.py FFF ======== 3 failed in 0.01 seconds=====
62. Get the tests passing again
63. from math import floor from flask import Flask, request app = Flask(__name__) @app.route('/celsius') def celsius(): try: fahrenheit = int(request.args.get('fahrenheit')) except TypeError: return 'No degrees in Fahrenheit provided.' celsius = (fahrenheit - 32) * 5 / 9; return str(floor(celsius))
64. =========== test session starts ======= test_py_api.py ... ======== 3 passed in 0.01 seconds=====
65. Reaching parity reduces risk.
66. the smaller the switch, the better.
67. IN REVIEW
68. Don’t treat code like a pile of trash. Clean it up as you go.
69. Remember the tube of toothpaste: Make the change easy, make the easy change.
70. If you have to jump across two trains, Get them to the same speed & switch.
71. We can change how we think about legacy code.
72. Technical debt
73. Technical wealth
74. Improving code pays dividends.
75. References: Refactoring - Martin Fowler & Kent Beck https://www.amazon.com/Refactoring-Improving-Design-Existing- Code/dp/B007WTFWJ6/ref=sr_1_1? s=books&ie=UTF8&qid=1472533198&sr=1-1&keywords=refactori ng+martin+fowler#nav-subnav Working with Legacy Code - Michael Feathers https://www.amazon.com/Working-Effectively-Legacy-Michael- Feathers/dp/0131177052 Forget Technical Debt — Here's How to Build Technical Wealth http://firstround.com/review/forget-technical-debt-heres-how-to- build-technical-wealth/
76. Credits: IMG_5163 - Justin Henry, https://flic.kr/p/ebaYR The sky is the limit #2 - Mark Dries, https://flic.kr/p/eeq2eQ The way it should be! - Patrick Dirden, https://flic.kr/p/ 8Yp4mf 0001_zoriah_photojournalist_war_photographer_20120719 _0631 - Zoriah, https://flic.kr/p/dkScaB Freaktography - Ravaged, https://flic.kr/p/nh72Tp Aachen - Aachener Dom Pala d’oro Antependium - Daniel Mennerich, https://flic.kr/p/pBmHyw
77. Thank you! Slides: http://bit.ly/twotrains www.kevinlondon.com @kevin_london