/
ruby-science.html
5085 lines (4836 loc) · 372 KB
/
ruby-science.html
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
373
374
375
376
377
378
379
380
381
382
383
384
385
386
387
388
389
390
391
392
393
394
395
396
397
398
399
400
401
402
403
404
405
406
407
408
409
410
411
412
413
414
415
416
417
418
419
420
421
422
423
424
425
426
427
428
429
430
431
432
433
434
435
436
437
438
439
440
441
442
443
444
445
446
447
448
449
450
451
452
453
454
455
456
457
458
459
460
461
462
463
464
465
466
467
468
469
470
471
472
473
474
475
476
477
478
479
480
481
482
483
484
485
486
487
488
489
490
491
492
493
494
495
496
497
498
499
500
501
502
503
504
505
506
507
508
509
510
511
512
513
514
515
516
517
518
519
520
521
522
523
524
525
526
527
528
529
530
531
532
533
534
535
536
537
538
539
540
541
542
543
544
545
546
547
548
549
550
551
552
553
554
555
556
557
558
559
560
561
562
563
564
565
566
567
568
569
570
571
572
573
574
575
576
577
578
579
580
581
582
583
584
585
586
587
588
589
590
591
592
593
594
595
596
597
598
599
600
601
602
603
604
605
606
607
608
609
610
611
612
613
614
615
616
617
618
619
620
621
622
623
624
625
626
627
628
629
630
631
632
633
634
635
636
637
638
639
640
641
642
643
644
645
646
647
648
649
650
651
652
653
654
655
656
657
658
659
660
661
662
663
664
665
666
667
668
669
670
671
672
673
674
675
676
677
678
679
680
681
682
683
684
685
686
687
688
689
690
691
692
693
694
695
696
697
698
699
700
701
702
703
704
705
706
707
708
709
710
711
712
713
714
715
716
717
718
719
720
721
722
723
724
725
726
727
728
729
730
731
732
733
734
735
736
737
738
739
740
741
742
743
744
745
746
747
748
749
750
751
752
753
754
755
756
757
758
759
760
761
762
763
764
765
766
767
768
769
770
771
772
773
774
775
776
777
778
779
780
781
782
783
784
785
786
787
788
789
790
791
792
793
794
795
796
797
798
799
800
801
802
803
804
805
806
807
808
809
810
811
812
813
814
815
816
817
818
819
820
821
822
823
824
825
826
827
828
829
830
831
832
833
834
835
836
837
838
839
840
841
842
843
844
845
846
847
848
849
850
851
852
853
854
855
856
857
858
859
860
861
862
863
864
865
866
867
868
869
870
871
872
873
874
875
876
877
878
879
880
881
882
883
884
885
886
887
888
889
890
891
892
893
894
895
896
897
898
899
900
901
902
903
904
905
906
907
908
909
910
911
912
913
914
915
916
917
918
919
920
921
922
923
924
925
926
927
928
929
930
931
932
933
934
935
936
937
938
939
940
941
942
943
944
945
946
947
948
949
950
951
952
953
954
955
956
957
958
959
960
961
962
963
964
965
966
967
968
969
970
971
972
973
974
975
976
977
978
979
980
981
982
983
984
985
986
987
988
989
990
991
992
993
994
995
996
997
998
999
1000
<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8">
<meta name="generator" content="pandoc">
<meta name="author" content="thoughtbot">
<meta name="author" content="Joe Ferris">
<meta name="author" content="Harlow Ward">
<title>Ruby Science</title>
<style type="text/css">code{white-space: pre;}</style>
<!--[if lt IE 9]>
<script src="http://html5shim.googlecode.com/svn/trunk/html5.js"></script>
<![endif]-->
<style type="text/css">
table.sourceCode, tr.sourceCode, td.lineNumbers, td.sourceCode {
margin: 0; padding: 0; vertical-align: baseline; border: none; }
table.sourceCode { width: 100%; line-height: 100%; }
td.lineNumbers { text-align: right; padding-right: 4px; padding-left: 4px; color: #aaaaaa; border-right: 1px solid #aaaaaa; }
td.sourceCode { padding-left: 5px; }
code > span.kw { color: #007020; font-weight: bold; }
code > span.dt { color: #902000; }
code > span.dv { color: #40a070; }
code > span.bn { color: #40a070; }
code > span.fl { color: #40a070; }
code > span.ch { color: #4070a0; }
code > span.st { color: #4070a0; }
code > span.co { color: #60a0b0; font-style: italic; }
code > span.ot { color: #007020; }
code > span.al { color: #ff0000; font-weight: bold; }
code > span.fu { color: #06287e; }
code > span.er { color: #ff0000; font-weight: bold; }
</style>
</head>
<body>
<header>
<h1 class="title">Ruby Science</h1>
<h2 class="author">thoughtbot</h2>
<h2 class="author">Joe Ferris</h2>
<h2 class="author">Harlow Ward</h2>
</header>
<nav id="TOC">
<ul>
<li><a href="#introduction">Introduction</a><ul>
<li><a href="#code-reviews">Code Reviews</a></li>
<li><a href="#follow-your-nose">Follow Your Nose</a></li>
<li><a href="#removing-resistance">Removing Resistance</a></li>
<li><a href="#bugs-and-churn">Bugs and Churn</a></li>
<li><a href="#tools-to-find-smells">Tools to Find Smells</a></li>
<li><a href="#how-to-read-this-book">How to Read This Book</a></li>
<li><a href="#example-application">Example Application</a></li>
</ul></li>
<li><a href="#long-method">Long Method</a><ul>
<li><a href="#symptoms">Symptoms</a></li>
<li><a href="#example">Example</a></li>
<li><a href="#solutions">Solutions</a></li>
</ul></li>
<li><a href="#large-class">Large Class</a><ul>
<li><a href="#symptoms-1">Symptoms</a></li>
<li><a href="#example-1">Example</a></li>
<li><a href="#solutions-1">Solutions</a></li>
<li><a href="#prevention">Prevention</a></li>
<li><a href="#private-methods">Private Methods</a></li>
<li><a href="#god-class">God Class</a></li>
</ul></li>
<li><a href="#feature-envy">Feature Envy</a><ul>
<li><a href="#symptoms-2">Symptoms</a></li>
<li><a href="#example-2">Example</a></li>
<li><a href="#solutions-2">Solutions</a></li>
<li><a href="#prevention-1">Prevention</a></li>
</ul></li>
<li><a href="#case-statement">Case Statement</a><ul>
<li><a href="#symptoms-3">Symptoms</a></li>
<li><a href="#type-codes">Type Codes</a><ul>
<li><a href="#example-3">Example</a></li>
<li><a href="#solutions-3">Solutions</a></li>
</ul></li>
</ul></li>
<li><a href="#shotgun-surgery">Shotgun Surgery</a><ul>
<li><a href="#symptoms-4">Symptoms</a></li>
<li><a href="#example-4">Example</a></li>
<li><a href="#solutions-4">Solutions</a></li>
<li><a href="#prevention-2">Prevention</a></li>
</ul></li>
<li><a href="#divergent-change">Divergent Change</a><ul>
<li><a href="#symptoms-5">Symptoms</a></li>
<li><a href="#example-5">Example</a></li>
<li><a href="#solutions-5">Solutions</a></li>
<li><a href="#prevention-3">Prevention</a></li>
</ul></li>
<li><a href="#long-parameter-list">Long Parameter List</a><ul>
<li><a href="#symptoms-6">Symptoms</a></li>
<li><a href="#example-6">Example</a></li>
<li><a href="#solutions-6">Solutions</a></li>
<li><a href="#anti-solution">Anti-Solution</a></li>
</ul></li>
<li><a href="#duplicated-code">Duplicated Code</a><ul>
<li><a href="#symptoms-7">Symptoms</a></li>
<li><a href="#example-7">Example</a></li>
<li><a href="#solutions-7">Solutions</a></li>
<li><a href="#prevention-4">Prevention</a></li>
</ul></li>
<li><a href="#uncommunicative-name">Uncommunicative Name</a><ul>
<li><a href="#symptoms-8">Symptoms</a></li>
<li><a href="#example-8">Example</a></li>
<li><a href="#solutions-8">Solutions</a></li>
</ul></li>
<li><a href="#single-table-inheritance-sti">Single Table Inheritance (STI)</a><ul>
<li><a href="#symptoms-9">Symptoms</a></li>
<li><a href="#example-9">Example</a></li>
<li><a href="#solutions-9">Solutions</a></li>
<li><a href="#prevention-5">Prevention</a></li>
</ul></li>
<li><a href="#comments">Comments</a><ul>
<li><a href="#symptoms-10">Symptoms</a></li>
<li><a href="#example-10">Example</a></li>
<li><a href="#solutions-10">Solutions</a></li>
</ul></li>
<li><a href="#mixin">Mixin</a><ul>
<li><a href="#symptoms-11">Symptoms</a></li>
<li><a href="#example-11">Example</a></li>
<li><a href="#solutions-11">Solutions</a></li>
<li><a href="#prevention-6">Prevention</a></li>
</ul></li>
<li><a href="#callback">Callback</a><ul>
<li><a href="#symptoms-12">Symptoms</a></li>
<li><a href="#example-12">Example</a></li>
<li><a href="#solutions-12">Solutions</a></li>
</ul></li>
<li><a href="#replace-conditional-with-polymorphism">Replace Conditional with Polymorphism</a><ul>
<li><a href="#uses">Uses</a></li>
<li><a href="#example-13">Example</a></li>
<li><a href="#replace-type-code-with-subclasses">Replace Type Code with Subclasses</a></li>
<li><a href="#single-table-inheritance-sti-1">Single Table Inheritance (STI)</a><ul>
<li><a href="#extracting-type-specific-code">Extracting Type-Specific Code</a></li>
</ul></li>
<li><a href="#polymorphic-partials">Polymorphic Partials</a><ul>
<li><a href="#multiple-polymorphic-views">Multiple Polymorphic Views</a></li>
<li><a href="#drawbacks">Drawbacks</a></li>
<li><a href="#next-steps">Next Steps</a></li>
</ul></li>
</ul></li>
<li><a href="#replace-conditional-with-null-object">Replace Conditional with Null Object</a><ul>
<li><a href="#uses-1">Uses</a></li>
<li><a href="#example-14">Example</a></li>
<li><a href="#drawbacks-1">Drawbacks</a></li>
<li><a href="#next-steps-1">Next Steps</a></li>
<li><a href="#truthiness-try-and-other-tricks">Truthiness, <code>try</code> and Other Tricks</a></li>
</ul></li>
<li><a href="#extract-method">Extract Method</a><ul>
<li><a href="#uses-2">Uses</a></li>
<li><a href="#example-15">Example</a></li>
<li><a href="#replace-temp-with-query">Replace Temp with Query</a><ul>
<li><a href="#other-examples">Other Examples</a></li>
<li><a href="#next-steps-2">Next Steps</a></li>
</ul></li>
</ul></li>
<li><a href="#rename-method">Rename Method</a><ul>
<li><a href="#uses-3">Uses</a></li>
<li><a href="#example-16">Example</a></li>
<li><a href="#next-steps-3">Next Steps</a></li>
</ul></li>
<li><a href="#extract-class">Extract Class</a><ul>
<li><a href="#uses-4">Uses</a></li>
<li><a href="#example-17">Example</a></li>
<li><a href="#drawbacks-2">Drawbacks</a></li>
<li><a href="#next-steps-4">Next Steps</a></li>
</ul></li>
<li><a href="#extract-value-object">Extract Value Object</a><ul>
<li><a href="#uses-5">Uses</a></li>
<li><a href="#example-18">Example</a></li>
<li><a href="#next-steps-5">Next Steps</a></li>
</ul></li>
<li><a href="#extract-decorator">Extract Decorator</a><ul>
<li><a href="#uses-6">Uses</a></li>
<li><a href="#example-19">Example</a></li>
<li><a href="#drawbacks-3">Drawbacks</a></li>
<li><a href="#next-steps-6">Next Steps</a></li>
</ul></li>
<li><a href="#extract-partial">Extract Partial</a><ul>
<li><a href="#uses-7">Uses</a></li>
<li><a href="#steps">Steps</a></li>
<li><a href="#example-20">Example</a></li>
<li><a href="#next-steps-7">Next Steps</a></li>
</ul></li>
<li><a href="#extract-validator">Extract Validator</a><ul>
<li><a href="#uses-8">Uses</a></li>
<li><a href="#example-21">Example</a></li>
<li><a href="#next-steps-8">Next Steps</a></li>
</ul></li>
<li><a href="#introduce-explaining-variable">Introduce Explaining Variable</a><ul>
<li><a href="#uses-9">Uses</a></li>
<li><a href="#example-22">Example</a></li>
<li><a href="#next-steps-9">Next Steps</a></li>
</ul></li>
<li><a href="#introduce-form-object">Introduce Form Object</a><ul>
<li><a href="#uses-10">Uses</a></li>
<li><a href="#example-23">Example</a></li>
<li><a href="#next-steps-10">Next Steps</a></li>
</ul></li>
<li><a href="#introduce-parameter-object">Introduce Parameter Object</a><ul>
<li><a href="#uses-11">Uses</a></li>
<li><a href="#example-24">Example</a></li>
<li><a href="#next-steps-11">Next Steps</a></li>
</ul></li>
<li><a href="#use-class-as-factory">Use Class as Factory</a><ul>
<li><a href="#uses-12">Uses</a></li>
<li><a href="#example-25">Example</a></li>
<li><a href="#next-steps-12">Next Steps</a></li>
</ul></li>
<li><a href="#move-method">Move Method</a><ul>
<li><a href="#uses-13">Uses</a></li>
<li><a href="#dangerous-move-and-extract-at-the-same-time">Dangerous: Move and Extract at the Same Time</a></li>
<li><a href="#next-steps-13">Next Steps</a></li>
</ul></li>
<li><a href="#inline-class">Inline Class</a><ul>
<li><a href="#uses-14">Uses</a></li>
<li><a href="#example-26">Example</a></li>
<li><a href="#drawbacks-4">Drawbacks</a></li>
<li><a href="#next-steps-14">Next Steps</a></li>
</ul></li>
<li><a href="#inject-dependencies">Inject Dependencies</a><ul>
<li><a href="#uses-15">Uses</a></li>
<li><a href="#example-27">Example</a></li>
<li><a href="#drawbacks-5">Drawbacks</a></li>
<li><a href="#next-steps-15">Next Steps</a></li>
</ul></li>
<li><a href="#replace-subclasses-with-strategies">Replace Subclasses with Strategies</a><ul>
<li><a href="#uses-16">Uses</a></li>
<li><a href="#example-28">Example</a></li>
<li><a href="#drawbacks-6">Drawbacks</a></li>
<li><a href="#next-steps-16">Next Steps</a></li>
</ul></li>
<li><a href="#replace-mixin-with-composition">Replace Mixin with Composition</a><ul>
<li><a href="#uses-17">Uses</a></li>
<li><a href="#example-29">Example</a></li>
<li><a href="#next-steps-17">Next Steps</a></li>
</ul></li>
<li><a href="#replace-callback-with-method">Replace Callback with Method</a><ul>
<li><a href="#uses-18">Uses</a></li>
<li><a href="#steps-1">Steps</a></li>
<li><a href="#example-30">Example</a></li>
<li><a href="#next-steps-18">Next Steps</a></li>
</ul></li>
<li><a href="#use-convention-over-configuration">Use Convention Over Configuration</a><ul>
<li><a href="#uses-19">Uses</a></li>
<li><a href="#example-31">Example</a></li>
<li><a href="#scoping-constantize">Scoping <code>constantize</code></a></li>
<li><a href="#drawbacks-7">Drawbacks</a></li>
</ul></li>
<li><a href="#dry">DRY</a><ul>
<li><a href="#duplicated-knowledge-vs.-duplicated-text">Duplicated Knowledge vs. Duplicated Text</a><ul>
<li><a href="#application">Application</a></li>
</ul></li>
</ul></li>
<li><a href="#single-responsibility-principle">Single Responsibility Principle</a><ul>
<li><a href="#reasons-to-change">Reasons to Change</a></li>
<li><a href="#stability">Stability</a></li>
<li><a href="#cohesion">Cohesion</a></li>
<li><a href="#responsibility-magnets">Responsibility Magnets</a></li>
<li><a href="#tension-with-tell-dont-ask">Tension with Tell, Don't Ask</a><ul>
<li><a href="#drawbacks-8">Drawbacks</a></li>
<li><a href="#application-1">Application</a></li>
</ul></li>
</ul></li>
<li><a href="#tell-dont-ask">Tell, Don't Ask</a><ul>
<li><a href="#encapsulation-of-logic">Encapsulation of Logic</a></li>
<li><a href="#encapsulation-of-state">Encapsulation of State</a></li>
<li><a href="#minimal-public-interface">Minimal Public Interface</a></li>
<li><a href="#tension-with-modelviewcontroller">Tension with Model—View—Controller</a><ul>
<li><a href="#application-2">Application</a></li>
</ul></li>
</ul></li>
<li><a href="#law-of-demeter">Law of Demeter</a><ul>
<li><a href="#multiple-dots">Multiple Dots</a></li>
<li><a href="#multiple-assignments">Multiple Assignments</a></li>
<li><a href="#the-spirit-of-the-law">The Spirit of the Law</a></li>
<li><a href="#objects-vs.-types">Objects vs. Types</a></li>
<li><a href="#duplication">Duplication</a><ul>
<li><a href="#application-3">Application</a></li>
</ul></li>
</ul></li>
<li><a href="#composition-over-inheritance">Composition Over Inheritance</a><ul>
<li><a href="#inheritance">Inheritance</a></li>
<li><a href="#composition">Composition</a></li>
<li><a href="#dynamic-vs.-static">Dynamic vs. Static</a></li>
<li><a href="#dynamic-inheritance">Dynamic Inheritance</a></li>
<li><a href="#the-trouble-with-hierarchies">The Trouble with Hierarchies</a></li>
<li><a href="#mixins">Mixins</a></li>
<li><a href="#single-table-inheritance">Single Table Inheritance</a><ul>
<li><a href="#drawbacks-9">Drawbacks</a></li>
<li><a href="#application-4">Application</a></li>
</ul></li>
</ul></li>
<li><a href="#openclosed-principle">Open/Closed Principle</a><ul>
<li><a href="#strategies">Strategies</a><ul>
<li><a href="#inheritance-1">Inheritance</a></li>
<li><a href="#decorators">Decorators</a></li>
<li><a href="#dependency-injection">Dependency Injection</a></li>
</ul></li>
<li><a href="#everything-is-open">Everything is Open</a></li>
<li><a href="#monkey-patching">Monkey Patching</a><ul>
<li><a href="#drawbacks-10">Drawbacks</a></li>
<li><a href="#application-5">Application</a></li>
</ul></li>
</ul></li>
<li><a href="#dependency-inversion-principle">Dependency Inversion Principle</a><ul>
<li><a href="#inversion-of-control">Inversion of Control</a></li>
<li><a href="#where-to-decide-dependencies">Where To Decide Dependencies</a><ul>
<li><a href="#drawbacks-11">Drawbacks</a></li>
<li><a href="#application-6">Application</a></li>
</ul></li>
</ul></li>
</ul>
</nav>
<p></p>
<section id="introduction" class="level1">
<h1><a href="#introduction">Introduction</a></h1>
<p>Ruby on Rails is almost a decade old, and its community has developed a number of principles for building applications that are fast, fun and easy to change: Don't repeat yourself, keep your views dumb, keep your controllers skinny, and keep business logic in your models. These principles carry most applications to their first release or beyond.</p>
<p>However, these principles only get you so far. After a few releases, most applications begin to suffer. Models become fat, classes become few and large, tests become slow and changes become painful. In many applications, there comes a day when the developers realize that there's no going back; the application is a twisted mess and the only way out is a rewrite or a new job.</p>
<p>Fortunately, it doesn't have to be this way. Developers have been using object-oriented programming for several decades and there's a wealth of knowledge out there that still applies to developing applications today. We can use the lessons learned by these developers to write good Rails applications by applying good object-oriented programming.</p>
<p>Ruby Science will outline a process for detecting emerging problems in code and will dive into the solutions, old and new.</p>
<section id="code-reviews" class="level2">
<h2><a href="#code-reviews">Code Reviews</a></h2>
<p>Our first step toward better code is to review it.</p>
<p>Have you ever sent an email with typos? Did you review what you wrote before clicking "Send"? Reviewing your e-mails prevents mistakes and reviewing your code does the same.</p>
<p>To make it easier to review code, always work in a feature branch. The branch reduces the temptation to push unreviewed code or to wait too long to push code.</p>
<p>The first person who should review every line of your code is you. Before committing new code, read each changed line. Use git's <code>diff</code> and <code>--patch</code> features to examine code before you commit. Read more about these features using <code>git help add</code> and <code>git help commit</code>.</p>
<p>If you're working on a team, push your feature branch and invite your teammates to review the changes via <code>git diff origin/master..HEAD</code>.</p>
<p>Team review reveals how understandable code is to someone other than the author. Your team members' understanding now is a good indicator of your understanding in the future.</p>
<p>However, what should you and your teammates look for during review?</p>
</section>
<section id="follow-your-nose" class="level2">
<h2><a href="#follow-your-nose">Follow Your Nose</a></h2>
<p>Code "smells" are indicators something may be wrong. They are useful because they are easy to see—sometimes easier than the root cause of a problem.</p>
<p>When you review code, watch for smells. Consider whether refactoring the code to remove the smell would result in better code. If you're reviewing a teammate's feature branch, share your best refactoring ideas with him or her.</p>
<p>Smells are associated with one or more refactorings (example: remove the Long Method smell using the Extract Method refactoring). Learn these associations in order to quickly consider them during review and whether the result (example: several small methods) improves the code.</p>
<p>Don't treat code smells as bugs. It will be a waste of time to "fix" every smell. Not every smell is the symptom of a problem and despite your best intentions, you can accidentally introduce another smell or problem.</p>
</section>
<section id="removing-resistance" class="level2">
<h2><a href="#removing-resistance">Removing Resistance</a></h2>
<p>Another opportunity for refactoring is when you're having difficulty making a change to existing code. This is called "resistance." The refactoring you choose depends on the type of resistance.</p>
<p>Is it hard to determine where new code belongs? The code is not readable enough. Rename methods and variables until it's obvious where your change belongs. This and every subsequent change will be easier. Refactor for readability first.</p>
<p>Is it hard to change the code without breaking existing code? Add extension points or extract code to be easier to reuse, and then try to introduce your change. Repeat this process until the change you want is easy to introduce.</p>
<p>Each change should be easy to introduce. If it's not, refactor.</p>
<p>When you are making your changes, you will be in a feature branch. Try to make your change without refactoring. If your meet resistance, make a "work in progress" commit, check out master and create a new refactoring branch:</p>
<pre><code>git commit -m 'wip: new feature'
git push
git checkout master
git checkout -b refactoring-for-new-feature</code></pre>
<p>Refactor until you fix the resistance you met on your feature branch. Then rebase your feature branch on top of your refactoring branch:</p>
<pre><code>git rebase -i new-feature
git checkout new-feature
git merge refactoring-for-new-feature --ff-only</code></pre>
<p>If the change is easier now, continue in your feature branch. If not, check out your refactoring branch and try again.</p>
</section>
<section id="bugs-and-churn" class="level2">
<h2><a href="#bugs-and-churn">Bugs and Churn</a></h2>
<p>If you're spending a lot of time swatting bugs, remove smells in the methods or classes of the buggy code. You'll make it less likely that a bug will be reintroduced.</p>
<p>After you commit a bug fix to a feature branch, find out if the code you changed to fix the bug is in files that change often. If the buggy code does change often, find smells and eliminate them. Separate the parts that change often from the parts that don't.</p>
<p>Conversely, avoid refactoring areas with low churn. Refactoring changes code and with each change, you risk introducing new bugs. If a file hasn't changed in six months, leave it alone. It may not be pretty, but you'll spend more time looking at it when you break it by trying to fix something that wasn't broken.</p>
</section>
<section id="tools-to-find-smells" class="level2">
<h2><a href="#tools-to-find-smells">Tools to Find Smells</a></h2>
<p>Some smells are easy to find while you're reading code and change sets, but other smells slip through the cracks without extra help.</p>
<p>Duplication is one of the hardest problems to find by hand. If you're using diffs during code reviews, it will be invisible when you copy and paste existing methods. The original method will be unchanged and won't show up in the diff, so unless the reviewer knows and remembers that the original existed, he or she won't notice that the copied method isn't just a new addition. Every duplicated piece of code is a bug waiting to happen.</p>
<p>Churn is similarly invisible, in that each change will look fine, and only the file's full history will reveal the smell.</p>
<p>Various tools are available which can aid you in your search for code smells.</p>
<p>Our favorite is <a href="https://codeclimate.com/">Code Climate</a>, which is a hosted tool and will scan your code for issues every time you push to Git. Code Climate attempts to locate hot spots for refactoring and assigns each class a simple A through F grade. It identifies complexity, duplication, churn and code smells.</p>
<p>If you're unable to use a hosted service, there are gems you can use locally, such as <a href="https://github.com/metricfu/metric_fu">metric_fu</a>, <a href="https://github.com/danmayer/churn">churn</a>, <a href="http://rubygems.org/gems/flog">flog</a>, <a href="http://rubygems.org/gems/flay">flay</a>, and <a href="https://github.com/troessner/reek/wiki">reek</a>. These gems can identify churn, complexity, duplication and smells.</p>
<p>Getting obsessed with the counts and scores from these tools will distract from the actual issues in your code, but it's worthwhile to run them continually and watch out for potential warning signs.</p>
</section>
<section id="how-to-read-this-book" class="level2">
<h2><a href="#how-to-read-this-book">How to Read This Book</a></h2>
<p>This book contains three catalogs: smells, solutions and principles.</p>
<p>Start by looking up a smell that sounds familiar. Each chapter on smells explains the potential problems each smell may reveal and references possible solutions.</p>
<p>Once you've identified the problem revealed by a smell, read the relevant solution chapter to learn how to fix it. Each solution chapter explains which problems it addresses and potential problems that can be introduced.</p>
<p>Lastly, smell and solution chapters reference related principles. The smell chapters reference principles that you can follow to avoid the root problem in the future. The solution chapters explain how each solution changes your code to follow related principles.</p>
<p>By following this process, you'll learn how to detect and fix actual problems in your code using smells and reusable solutions, and you'll learn about principles that you can follow to improve the code you write from the beginning.</p>
</section>
<section id="example-application" class="level2">
<h2><a href="#example-application">Example Application</a></h2>
<p>This book comes with a bundled example application, which you can find <a href="https://github.com/thoughtbot/ruby-science/tree/master/example_app">on GitHub</a>. Make sure that you sign into GitHub before attempting to view example application and commit links, or you'll receive a 404 error.</p>
<p>Most of the code samples included in the book come directly from commits in the example application. Several chapters link to the full commits for related changes. At any point, you can check out the application locally and check out those commits to explore solutions in progress.</p>
<p>For some solutions, the entire change is not included in the chapter for the sake of focus and brevity. However, you can see every change made for a solution in the example commits, including tests.</p>
<p>Make sure to take a look at the application's <a href="https://github.com/thoughtbot/ruby-science/blob/master/example_app/README.md">README</a>, as it contains a summary of the application and instructions for setting it up.</p>
<p></p>
</section>
</section>
<section id="long-method" class="level1">
<h1><a href="#long-method">Long Method</a></h1>
<p>The most common smell in Rails applications is the Long Method.</p>
<p>Long methods are exactly what they sound like: methods that are too long. They're easy to spot.</p>
<section id="symptoms" class="level3">
<h3><a href="#symptoms">Symptoms</a></h3>
<ul>
<li>If you can't tell exactly what a method does at a glance, it's too long.</li>
<li>Methods with more than one level of nesting are usually too long.</li>
<li>Methods with more than one level of abstraction may be too long.</li>
<li>Methods with a flog score of 10 or higher may be too long.</li>
</ul>
<p>You can watch out for long methods as you write them, but finding existing methods is easiest with tools like flog:</p>
<pre><code>% flog app lib
72.9: flog total
5.6: flog/method average
15.7: QuestionsController#create app/controllers/questions_controller.rb:9
11.7: QuestionsController#new app/controllers/questions_controller.rb:2
11.0: Question#none
8.1: SurveysController#create app/controllers/surveys_controller.rb:6</code></pre>
<p>Methods with higher scores are more complicated. Anything with a score higher than 10 is worth looking at, but flog only helps you find potential trouble spots; use your own judgment when refactoring.</p>
</section>
<section id="example" class="level3">
<h3><a href="#example">Example</a></h3>
<p>For an example of a long method, let's take a look at the highest scored method from flog, <code>QuestionsController#create</code>:</p>
<pre class="sourceCode ruby"><code class="sourceCode ruby"><span class="kw">def</span> create
<span class="ot">@survey</span> = <span class="dt">Survey</span>.find(params[<span class="st">:survey_id</span>])
<span class="ot">@submittable_type</span> = params[<span class="st">:submittable_type_id</span>]
question_params = params.
require(<span class="st">:question</span>).
permit(<span class="st">:submittable_type</span>, <span class="st">:title</span>, <span class="st">:options_attributes</span>, <span class="st">:minimum</span>, <span class="st">:maximum</span>)
<span class="ot">@question</span> = <span class="ot">@survey</span>.questions.new(question_params)
<span class="ot">@question</span>.submittable_type = <span class="ot">@submittable_type</span>
<span class="kw">if</span> <span class="ot">@question</span>.save
redirect_to <span class="ot">@survey</span>
<span class="kw">else</span>
render <span class="st">:new</span>
<span class="kw">end</span>
<span class="kw">end</span></code></pre>
</section>
<section id="solutions" class="level3">
<h3><a href="#solutions">Solutions</a></h3>
<ul>
<li><a href="#extract-method">Extract method</a> is the most common way to break apart long methods.</li>
<li><a href="#replace-temp-with-query">Replace temp with query</a> if you have local variables in the method.</li>
</ul>
<p>After extracting methods, check for <a href="#feature-envy">feature envy</a> in the new methods to see if you should employ <a href="#move-method">move method</a> to provide the method with a better home.</p>
</section>
</section>
<section id="large-class" class="level1">
<h1><a href="#large-class">Large Class</a></h1>
<p>Most Rails applications suffer from several Large Classes. Large classes are difficult to understand and make it harder to change or reuse behavior. Tests for large classes are slow and churn tends to be higher, leading to more bugs and conflicts. Large classes likely also suffer from <a href="#divergent-change">divergent change</a>.</p>
<section id="symptoms-1" class="level3">
<h3><a href="#symptoms-1">Symptoms</a></h3>
<ul>
<li>You can't easily describe what the class does in one sentence.</li>
<li>You can't tell what the class does without scrolling.</li>
<li>The class needs to change for more than one reason.</li>
<li>The class has more than seven methods.</li>
<li>The class has a total flog score of 50.</li>
</ul>
</section>
<section id="example-1" class="level3">
<h3><a href="#example-1">Example</a></h3>
<p>This class has a high flog score, has a large number of methods, more private than public methods and has multiple responsibilities:</p>
<pre class="sourceCode ruby"><code class="sourceCode ruby"><span class="co"># app/models/question.rb</span>
<span class="kw">class</span> <span class="dt">Question</span> < <span class="dt">ActiveRecord</span>::<span class="dt">Base</span>
<span class="kw">include</span> <span class="dt">ActiveModel</span>::<span class="dt">ForbiddenAttributesProtection</span>
<span class="dt">SUBMITTABLE_TYPES</span> =<span class="ot"> %w(</span><span class="st">Open MultipleChoice Scale</span><span class="ot">)</span>.freeze
validates <span class="st">:maximum</span>, presence: <span class="dv">true</span>, <span class="kw">if</span>: <span class="st">:scale?</span>
validates <span class="st">:minimum</span>, presence: <span class="dv">true</span>, <span class="kw">if</span>: <span class="st">:scale?</span>
validates <span class="st">:question_type</span>, presence: <span class="dv">true</span>, inclusion: <span class="dt">SUBMITTABLE_TYPES</span>
validates <span class="st">:title</span>, presence: <span class="dv">true</span>
belongs_to <span class="st">:survey</span>
has_many <span class="st">:answers</span>
has_many <span class="st">:options</span>
accepts_nested_attributes_for <span class="st">:options</span>, reject_if: <span class="st">:all_blank</span>
<span class="kw">def</span> summary
<span class="kw">case</span> question_type
<span class="kw">when</span> <span class="st">'MultipleChoice'</span>
summarize_multiple_choice_answers
<span class="kw">when</span> <span class="st">'Open'</span>
summarize_open_answers
<span class="kw">when</span> <span class="st">'Scale'</span>
summarize_scale_answers
<span class="kw">end</span>
<span class="kw">end</span>
<span class="kw">def</span> steps
(minimum..maximum).to_a
<span class="kw">end</span>
<span class="kw">private</span>
<span class="kw">def</span> scale?
question_type == <span class="st">'Scale'</span>
<span class="kw">end</span>
<span class="kw">def</span> summarize_multiple_choice_answers
total = answers.count
counts = answers.group(<span class="st">:text</span>).order(<span class="st">'COUNT(*) DESC'</span>).count
percents = counts.map <span class="kw">do</span> |text, count|
percent = (<span class="fl">100.0</span> * count / total).round
<span class="st">"</span><span class="ot">#{</span>percent<span class="ot">}</span><span class="st">% </span><span class="ot">#{</span>text<span class="ot">}</span><span class="st">"</span>
<span class="kw">end</span>
percents.join(<span class="st">', '</span>)
<span class="kw">end</span>
<span class="kw">def</span> summarize_open_answers
answers.order(<span class="st">:created_at</span>).pluck(<span class="st">:text</span>).join(<span class="st">', '</span>)
<span class="kw">end</span>
<span class="kw">def</span> summarize_scale_answers
sprintf(<span class="st">'Average: %.02f'</span>, answers.average(<span class="st">'text'</span>))
<span class="kw">end</span>
<span class="kw">end</span></code></pre>
</section>
<section id="solutions-1" class="level3">
<h3><a href="#solutions-1">Solutions</a></h3>
<ul>
<li><a href="#move-method">Move method</a> to move methods to another class if an existing class could better handle the responsibility.</li>
<li><a href="#extract-class">Extract class</a> if the class has multiple responsibilities.</li>
<li><a href="#replace-conditional-with-polymorphism">Replace conditional with polymorphism</a> if the class contains private methods related to conditional branches.</li>
<li><a href="#extract-value-object">Extract value object</a> if the class contains private query methods.</li>
<li><a href="#extract-decorator">Extract decorator</a> if the class contains delegation methods.</li>
<li><a href="#replace-subclasses-with-strategies">Replace subclasses with strategies</a> if the large class is a base class in an inheritance hierarchy.</li>
</ul>
</section>
<section id="prevention" class="level3">
<h3><a href="#prevention">Prevention</a></h3>
<p>Following the <a href="#single-responsibility-principle">single responsibility principle</a> will prevent large classes from cropping up. It's difficult for any class to become too large without taking on more than one responsibility.</p>
<p>Using <a href="#composition-over-inheritance">composition over inheritance</a> makes it easier to create small classes.</p>
<p>If a large portion of the class is devoted to instantiating subclasses, try following the <a href="#dependency-inversion-principle">dependency inversion principle</a>.</p>
<p>Following the <a href="#openclosed-principle">open/closed principle</a> will prevent large classes by preventing new concerns from being introduced.</p>
<p>You can use flog to analyze classes as you write and modify them:</p>
<pre><code>% flog -a app/models/question.rb
48.3: flog total
6.9: flog/method average
15.6: Question#summarize_multiple_choice_answers app/models/question.rb:38
12.0: Question#none
6.3: Question#summary app/models/question.rb:17
5.2: Question#summarize_open_answers app/models/question.rb:48
3.6: Question#summarize_scale_answers app/models/question.rb:52
3.4: Question#steps app/models/question.rb:28
2.2: Question#scale? app/models/question.rb:34</code></pre>
</section>
<section id="private-methods" class="level2">
<h2><a href="#private-methods">Private Methods</a></h2>
<p>In general, public methods are a greater liability than private methods. This is because it's harder to tell where public methods are used, so you need to take greater care when refactoring them. However, a large suite of private methods is also a strong indicator of a large class.</p>
<p>Private methods can't be reused between classes, which makes it more likely that code will be duplicated. Extracting private methods to new classes makes it easier for developers to do the right thing.</p>
<p>Additionally, private methods can't be tested directly. This makes it more difficult to write focused, simple unit tests, since the tests will need to go through one or more public methods. The further a test is from the code it tests, the harder it is to understand.</p>
<p>Lastly, private methods are often the easiest to extract to new classes. Large classes can be difficult to split up because of entangled dependencies between public and private methods.</p>
<p>Attempts to extract public methods will frequently halt when shared dependencies are discovered on private methods. Extracting the private behavior of a class into a small, reusable class is often the easiest first step towards splitting up a large class.</p>
<p>Keeping a class's public interface as small as possible is a best practice. However, keep an eye on your private interface as well. A maze of private dependencies is a good sign that your public interface is not cohesive and can be split into two or more classes.</p>
</section>
<section id="god-class" class="level2">
<h2><a href="#god-class">God Class</a></h2>
<p>A particular specimen of large class affects most Rails applications: the God class. A God class is any class that seems to know everything about an application. It has a reference to the majority of the other models and it's difficult to answer any question or perform any action in the application without going through this class.</p>
<p>Most applications have two God classes: the user, and the central focus of the application. For a todo list application, it will be user and todo; for photo sharing application, it will be user and photo.</p>
<p>You need to be particularly vigilant about refactoring these classes. If you don't start splitting up your God classes early on, it will become impossible to separate them without rewriting most of your application.</p>
<p>Treatment and prevention of God classes is the same as for any large class.</p>
</section>
</section>
<section id="feature-envy" class="level1">
<h1><a href="#feature-envy">Feature Envy</a></h1>
<p>Feature envy reveals a method (or method-to-be) that would work better on a different class.</p>
<p>Methods suffering from feature envy contain logic that is difficult to reuse because the logic is trapped within a method on the wrong class. These methods are also often private methods, which makes them unavailable to other classes. Moving the method (or the affected portion of a method) to a more appropriate class improves readability, makes the logic easier to reuse and reduces coupling.</p>
<section id="symptoms-2" class="level3">
<h3><a href="#symptoms-2">Symptoms</a></h3>
<ul>
<li>Repeated references to the same object.</li>
<li>Parameters or local variables that are used more than methods and instance variables of the class in question.</li>
<li>Methods that include a class name in their own names (such as <code>invite_user</code>).</li>
<li>Private methods on the same class that accept the same parameter.</li>
<li><a href="#law-of-demeter">Law of Demeter</a> violations.</li>
<li><a href="#tell-dont-ask">Tell, don't ask</a> violations.</li>
</ul>
</section>
<section id="example-2" class="level3">
<h3><a href="#example-2">Example</a></h3>
<pre class="sourceCode ruby"><code class="sourceCode ruby"><span class="co"># app/models/completion.rb</span>
<span class="kw">def</span> score
answers.inject(<span class="dv">0</span>) <span class="kw">do</span> |result, answer|
question = answer.question
result + question.score(answer.text)
<span class="kw">end</span>
<span class="kw">end</span></code></pre>
<p>The <code>answer</code> local variable is used twice in the block: once to get its <code>question</code>, and once to get its <code>text</code>. This tells us that we can probably extract a new method and move it to the <code>answer</code> class.</p>
</section>
<section id="solutions-2" class="level3">
<h3><a href="#solutions-2">Solutions</a></h3>
<ul>
<li><a href="#extract-method">Extract method</a> if only part of the method suffers from feature envy; then move the method.</li>
<li><a href="#move-method">Move method</a> if the entire method suffers from feature envy.</li>
<li><a href="#inline-class">Inline class</a> if the envied class isn't pulling its weight.</li>
</ul>
</section>
<section id="prevention-1" class="level3">
<h3><a href="#prevention-1">Prevention</a></h3>
<p>Following the <a href="#law-of-demeter">law of Demeter</a> will prevent a lot of feature envy by limiting the dependencies of each method.</p>
<p>Following <a href="#tell-dont-ask">tell, don't ask</a> will prevent feature envy by avoiding unnecessary inspection of another object's state.</p>
</section>
</section>
<section id="case-statement" class="level1">
<h1><a href="#case-statement">Case Statement</a></h1>
<p>Case Statements are a sign that a method contains too much knowledge.</p>
<section id="symptoms-3" class="level3">
<h3><a href="#symptoms-3">Symptoms</a></h3>
<ul>
<li>Case statements that check the class of an object.</li>
<li>Case statements that check a type code.</li>
<li><a href="#divergent-change">Divergent change</a> caused by changing or adding <code>when</code> clauses.</li>
<li><a href="#shotgun-surgery">Shotgun surgery</a> caused by duplicating the case statement.</li>
</ul>
<p>Actual <code>case</code> statements are extremely easy to find. Just grep your codebase for "case." However, you should also be on the lookout for <code>case</code>'s sinister cousin, the repetitive <code>if-elsif</code>.</p>
</section>
<section id="type-codes" class="level2">
<h2><a href="#type-codes">Type Codes</a></h2>
<p>Some applications contain type codes—fields that store type information about objects. These fields are easy to add and seem innocent, but result in code that's harder to maintain. A better solution is to take advantage of Ruby's ability to invoke different behavior based on an object's class, called "dynamic dispatch." Using a case statement with a type code inelegantly reproduces dynamic dispatch.</p>
<p>The special <code>type</code> column that ActiveRecord uses is not necessarily a type code. The <code>type</code> column is used to serialize an object's class to the database so that the correct class can be instantiated later on. If you're just using the <code>type</code> column to let ActiveRecord decide which class to instantiate, this isn't a smell. However, make sure to avoid referencing the <code>type</code> column from <code>case</code> or <code>if</code> statements.</p>
<section id="example-3" class="level3">
<h3><a href="#example-3">Example</a></h3>
<p>This method summarizes the answers to a question. The summary varies based on the type of question.</p>
<pre class="sourceCode ruby"><code class="sourceCode ruby"><span class="co"># app/models/question.rb</span>
<span class="kw">def</span> summary
<span class="kw">case</span> question_type
<span class="kw">when</span> <span class="st">'MultipleChoice'</span>
summarize_multiple_choice_answers
<span class="kw">when</span> <span class="st">'Open'</span>
summarize_open_answers
<span class="kw">when</span> <span class="st">'Scale'</span>
summarize_scale_answers
<span class="kw">end</span>
<span class="kw">end</span></code></pre>
<p></p>
<p>Note that many applications replicate the same <code>case</code> statement, which is a more serious offence. This view duplicates the <code>case</code> logic from <code>Question#summary</code>, this time in the form of multiple <code>if</code> statements:</p>
<pre class="sourceCode rhtml"><code class="sourceCode rhtml"># app/views/questions/_question.html.erb
<span class="kw"><%</span> <span class="kw">if</span> question.question_type <span class="ch">==</span> <span class="st">'MultipleChoice'</span> <span class="kw">-%></span>
<span class="kw"><ol></span>
<span class="kw"><%</span> question.options.each <span class="kw">do</span> <span class="ch">|</span>option<span class="ch">|</span> <span class="kw">-%></span>
<span class="kw"><li></span>
<span class="kw"><%=</span> submission_fields.radio_button <span class="st">:text</span>, option.text, id: dom_id(option) <span class="kw">%></span>
<span class="kw"><%=</span> content_tag <span class="st">:label</span>, option.text, <span class="kw">for</span>: dom_id(option) <span class="kw">%></span>
<span class="kw"></li></span>
<span class="kw"><%</span> <span class="kw">end</span> <span class="kw">-%></span>
<span class="kw"></ol></span>
<span class="kw"><%</span> <span class="kw">end</span> <span class="kw">-%></span>
<span class="kw"><%</span> <span class="kw">if</span> question.question_type <span class="ch">==</span> <span class="st">'Scale'</span> <span class="kw">-%></span>
<span class="kw"><ol></span>
<span class="kw"><%</span> question.steps.each <span class="kw">do</span> <span class="ch">|</span>step<span class="ch">|</span> <span class="kw">-%></span>
<span class="kw"><li></span>
<span class="kw"><%=</span> submission_fields.radio_button <span class="st">:text</span>, step <span class="kw">%></span>
<span class="kw"><%=</span> submission_fields.label <span class="st">"text_</span><span class="ot">#{</span>step<span class="ot">}</span><span class="st">"</span>, label: step <span class="kw">%></span>
<span class="kw"></li></span>
<span class="kw"><%</span> <span class="kw">end</span> <span class="kw">-%></span>
<span class="kw"></ol></span>
<span class="kw"><%</span> <span class="kw">end</span> <span class="kw">-%></span></code></pre>
</section>
<section id="solutions-3" class="level3">
<h3><a href="#solutions-3">Solutions</a></h3>
<ul>
<li><a href="#replace-type-code-with-subclasses">Replace type code with subclasses</a> if the <code>case</code> statement is checking a type code, such as <code>question_type</code>.</li>
<li><a href="#replace-conditional-with-polymorphism">Replace conditional with polymorphism</a> when the <code>case</code> statement is checking the class of an object.</li>
<li><a href="#use-convention-over-configuration">Use convention over configuration</a> when selecting a strategy based on a string name.</li>
</ul>
</section>
</section>
</section>
<section id="shotgun-surgery" class="level1">
<h1><a href="#shotgun-surgery">Shotgun Surgery</a></h1>
<p>Shotgun Surgery is usually a more obvious symptom that reveals another smell.</p>
<section id="symptoms-4" class="level3">
<h3><a href="#symptoms-4">Symptoms</a></h3>
<ul>
<li>You have to make the same small change across several different files.</li>
<li>Changes become difficult to manage because they are hard to keep track of.</li>
</ul>
<p>Make sure you look for related smells in the affected code:</p>
<ul>
<li><a href="#duplicated-code">Duplicated code</a></li>
<li><a href="#case-statement">Case statement</a></li>
<li><a href="#feature-envy">Feature envy</a></li>
<li><a href="#long-parameter-list">Long parameter list</a></li>
</ul>
</section>
<section id="example-4" class="level3">
<h3><a href="#example-4">Example</a></h3>
<p>Users' names are formatted and displayed as "First Last" throughout the application. If you want to change the formatting to include a middle initial (example: "First M. Last") you'll need to make the same small change in several places.</p>
<pre class="sourceCode rhtml"><code class="sourceCode rhtml"># app/views/users/show.html.erb
<span class="kw"><%=</span> current_user.first_name <span class="kw">%></span> <span class="kw"><%=</span> current_user.last_name <span class="kw">%></span>
# app/views/users/index.html.erb
<span class="kw"><%=</span> current_user.first_name <span class="kw">%></span> <span class="kw"><%=</span> current_user.last_name <span class="kw">%></span>
# app/views/layouts/application.html.erb
<span class="kw"><%=</span> current_user.first_name <span class="kw">%></span> <span class="kw"><%=</span> current_user.last_name <span class="kw">%></span>
# app/views/mailers/completion_notification.html.erb
<span class="kw"><%=</span> current_user.first_name <span class="kw">%></span> <span class="kw"><%=</span> current_user.last_name <span class="kw">%></span></code></pre>
</section>
<section id="solutions-4" class="level3">
<h3><a href="#solutions-4">Solutions</a></h3>
<ul>
<li><a href="#replace-conditional-with-polymorphism">Replace conditional with polymorphism</a> to replace duplicated <code>case</code> statements and <code>if-elsif</code> blocks.</li>
<li><a href="#replace-conditional-with-null-object">Replace conditional with null object</a> if changing a method to return <code>nil</code> would require checks for <code>nil</code> in several places.</li>
<li><a href="#extract-decorator">Extract decorator</a> to replace duplicated display code in views/templates.</li>
<li><a href="#introduce-parameter-object">Introduce parameter object</a> to hang useful formatting methods alongside a data clump of related attributes.</li>
<li><a href="#use-convention-over-configuration">Use convention over configuration</a> to eliminate small steps that can be inferred based on a convention, such as a name.</li>
<li><a href="#inline-class">Inline class</a> if the class only serves to add extra steps when performing changes.</li>
</ul>
</section>
<section id="prevention-2" class="level3">
<h3><a href="#prevention-2">Prevention</a></h3>
<p>If your changes become spread out because you need to pass information between boundaries for dependencies, try <a href="#dependency-inversion-principle">inverting control</a>.</p>
<p>If you find yourself repeating the exact same change in several places, make sure that you <a href="#dry">Don't Repeat Yourself</a>.</p>
<p>If you need to change several places because of a modification in your dependency chain, such as changing <code>user.plan.price</code> to <code>user.account.plan.price</code>, make sure that you're following the <a href="#law-of-demeter">law of Demeter</a>.</p>
<p>If conditional logic is affected in several places by a single, cohesive change, make sure that you're following <a href="#tell-dont-ask">tell, don't ask</a>.</p>
</section>
</section>
<section id="divergent-change" class="level1">
<h1><a href="#divergent-change">Divergent Change</a></h1>
<p>A class suffers from Divergent Change when it changes for multiple reasons.</p>
<section id="symptoms-5" class="level3">
<h3><a href="#symptoms-5">Symptoms</a></h3>
<ul>
<li>You can't easily describe what the class does in one sentence.</li>
<li>The class is changed more frequently than other classes in the application.</li>
<li>Different changes to the class aren't related to each other.</li>
</ul>
<p></p>
</section>
<section id="example-5" class="level3">
<h3><a href="#example-5">Example</a></h3>
<pre class="sourceCode ruby"><code class="sourceCode ruby"><span class="co"># app/controllers/summaries_controller.rb</span>
<span class="kw">class</span> <span class="dt">SummariesController</span> < <span class="dt">ApplicationController</span>
<span class="kw">def</span> show
<span class="ot">@survey</span> = <span class="dt">Survey</span>.find(params[<span class="st">:survey_id</span>])
<span class="ot">@summaries</span> = <span class="ot">@survey</span>.summarize(summarizer)
<span class="kw">end</span>
<span class="kw">private</span>
<span class="kw">def</span> summarizer
<span class="kw">case</span> params[<span class="st">:id</span>]
<span class="kw">when</span> <span class="st">'breakdown'</span>
<span class="dt">Breakdown</span>.new
<span class="kw">when</span> <span class="st">'most_recent'</span>
<span class="dt">MostRecent</span>.new
<span class="kw">when</span> <span class="st">'your_answers'</span>
<span class="dt">UserAnswer</span>.new(current_user)
<span class="kw">else</span>
raise <span class="st">"Unknown summary type: </span><span class="ot">#{</span>params[<span class="st">:id</span>]<span class="ot">}</span><span class="st">"</span>
<span class="kw">end</span>
<span class="kw">end</span>
<span class="kw">end</span></code></pre>
<p>This controller has many reasons to change:</p>
<ul>
<li>Control flow logic related to summaries, such as authentication.</li>
<li>Any instance in which a summarizer strategy is added or changed.</li>
</ul>
</section>
<section id="solutions-5" class="level3">
<h3><a href="#solutions-5">Solutions</a></h3>
<ul>
<li><a href="#extract-class">Extract class</a> to move one cause of change to a new class.</li>
<li><a href="#move-method">Move method</a> if the class is changing because of methods that relate to another class.</li>
<li><a href="#extract-validator">Extract validator</a> to move validation logic out of models.</li>
<li><a href="#introduce-form-object">Introduce form object</a> to move form logic out of controllers.</li>
<li><a href="#use-convention-over-configuration">Use convention over configuration</a> to eliminate changes that can be inferred by a convention, such as a name.</li>
</ul>
</section>
<section id="prevention-3" class="level3">
<h3><a href="#prevention-3">Prevention</a></h3>
<p>You can prevent divergent change from occurring by following the <a href="#single-responsibility-principle">single responsibility principle</a>. If a class has only one responsibility, it has only one reason to change.</p>
<p>Following the <a href="#openclosed-principle">open/closed principle</a> limits future changes to classes, including divergent change.</p>
<p>Following <a href="#composition-over-inheritance">composition over inheritance</a> will make it easier to create small classes, preventing divergent change.</p>
<p>If a large portion of the class is devoted to instantiating subclasses, try following the <a href="#dependency-inversion-principle">dependency inversion principle</a>.</p>
<p>You can use churn to discover which files are changing most frequently. This isn't a direct relationship, but frequently changed files often have more than one responsibility and thus more than one reason to change.</p>
</section>
</section>
<section id="long-parameter-list" class="level1">
<h1><a href="#long-parameter-list">Long Parameter List</a></h1>
<p>Ruby supports positional method arguments which can lead to Long Parameter Lists.</p>
<section id="symptoms-6" class="level3">
<h3><a href="#symptoms-6">Symptoms</a></h3>
<ul>
<li>You can't easily change the method's arguments.</li>
<li>The method has three or more arguments.</li>
<li>The method is complex due to number of collaborating parameters.</li>
<li>The method requires large amounts of setup during isolated testing.</li>
</ul>
<p></p>
</section>
<section id="example-6" class="level3">
<h3><a href="#example-6">Example</a></h3>
<p>Look at this mailer for an example of long parameter list.</p>
<pre class="sourceCode ruby"><code class="sourceCode ruby"><span class="co"># app/mailers/mailer.rb</span>
<span class="kw">class</span> <span class="dt">Mailer</span> < <span class="dt">ActionMailer</span>::<span class="dt">Base</span>
default from: <span class="st">"from@example.com"</span>
<span class="kw">def</span> completion_notification(first_name, last_name, email)
<span class="ot">@first_name</span> = first_name
<span class="ot">@last_name</span> = last_name
mail(
to: email,
subject: <span class="st">'Thank you for completing the survey'</span>
)
<span class="kw">end</span>
<span class="kw">end</span></code></pre>
</section>
<section id="solutions-6" class="level3">
<h3><a href="#solutions-6">Solutions</a></h3>
<ul>
<li><p><a href="#introduce-parameter-object">Introduce parameter object</a> and pass it in as an object of naturally grouped attributes.</p></li>
<li><p><a href="#extract-class">Extract class</a> if the method is complex due to the number of collaborators.</p></li>
</ul>
</section>
<section id="anti-solution" class="level3">
<h3><a href="#anti-solution">Anti-Solution</a></h3>
<p>A common technique used to mask a long parameter list is grouping parameters using a hash of named parameters; this will replace connascence of position with connascence of name (a good first step). However, it will not reduce the number of collaborators in the method.</p>
</section>
</section>
<section id="duplicated-code" class="level1">
<h1><a href="#duplicated-code">Duplicated Code</a></h1>
<p>One of the first principles we're taught as developers: <a href="#dry">Don't Repeat Yourself</a>.</p>
<section id="symptoms-7" class="level3">
<h3><a href="#symptoms-7">Symptoms</a></h3>
<ul>
<li>You find yourself copying and pasting code from one place to another.</li>
<li><a href="#shotgun-surgery">Shotgun surgery</a> occurs when changes to your application require the same small edits in multiple places.</li>
</ul>
<p></p>
</section>
<section id="example-7" class="level3">
<h3><a href="#example-7">Example</a></h3>
<p>The <code>QuestionsController</code> suffers from duplication in the <code>create</code> and <code>update</code> methods.</p>
<pre class="sourceCode ruby"><code class="sourceCode ruby"><span class="co"># app/controllers/questions_controller.rb</span>
<span class="kw">def</span> create
<span class="ot">@survey</span> = <span class="dt">Survey</span>.find(params[<span class="st">:survey_id</span>])
question_params = params.
require(<span class="st">:question</span>).
permit(<span class="st">:title</span>, <span class="st">:options_attributes</span>, <span class="st">:minimum</span>, <span class="st">:maximum</span>)
<span class="ot">@question</span> = type.constantize.new(question_params)
<span class="ot">@question</span>.survey = <span class="ot">@survey</span>
<span class="kw">if</span> <span class="ot">@question</span>.save
redirect_to <span class="ot">@survey</span>
<span class="kw">else</span>
render <span class="st">:new</span>
<span class="kw">end</span>
<span class="kw">end</span>
<span class="kw">def</span> update
<span class="ot">@question</span> = <span class="dt">Question</span>.find(params[<span class="st">:id</span>])
question_params = params.
require(<span class="st">:question</span>).
permit(<span class="st">:title</span>, <span class="st">:options_attributes</span>, <span class="st">:minimum</span>, <span class="st">:maximum</span>)
<span class="ot">@question</span>.update_attributes(question_params)
<span class="kw">if</span> <span class="ot">@question</span>.save
redirect_to <span class="ot">@question</span>.survey
<span class="kw">else</span>
render <span class="st">:edit</span>
<span class="kw">end</span>
<span class="kw">end</span></code></pre>
</section>
<section id="solutions-7" class="level3">
<h3><a href="#solutions-7">Solutions</a></h3>
<ul>
<li><a href="#extract-method">Extract method</a> for duplicated code in the same file.</li>
<li><a href="#extract-class">Extract class</a> for duplicated code across multiple files.</li>
<li><a href="#extract-partial">Extract partial</a> for duplicated view and template code.</li>
<li><a href="#replace-conditional-with-polymorphism">Replace conditional with polymorphism</a> for duplicated conditional logic.</li>
<li><a href="#replace-conditional-with-null-object">Replace conditional with null object</a> to remove duplicated checks for <code>nil</code> values.</li>
</ul>
</section>
<section id="prevention-4" class="level3">
<h3><a href="#prevention-4">Prevention</a></h3>
<p>Following the <a href="#single-responsibility-principle">single responsibility principle</a> will result in small classes that are easier to reuse, reducing the temptation of duplication.</p>
</section>
</section>
<section id="uncommunicative-name" class="level1">
<h1><a href="#uncommunicative-name">Uncommunicative Name</a></h1>
<p>Software is run by computers—but written and read by humans. Names provide important information to developers who are trying to understand a piece of code. Patterns and challenges when naming a method or class can also provide clues for refactoring.</p>
<section id="symptoms-8" class="level3">
<h3><a href="#symptoms-8">Symptoms</a></h3>
<ul>
<li>Difficulty understanding a method or class.</li>
<li>Methods or classes with similar names but dissimilar functionality.</li>
<li>Redundant names, such as names that include the type of object to which they refer.</li>
</ul>
</section>
<section id="example-8" class="level3">
<h3><a href="#example-8">Example</a></h3>
<p>In our example application, the <code>SummariesController</code> generates summaries from a <code>Survey</code>:</p>
<pre class="sourceCode ruby"><code class="sourceCode ruby"><span class="co"># app/controllers/summaries_controller.rb</span>
<span class="ot">@summaries</span> = <span class="ot">@survey</span>.summarize(summarizer)</code></pre>
<p>The <code>summarize</code> method on <code>Survey</code> asks each <code>Question</code> to <code>summarize</code> itself using a <code>summarizer</code>:</p>
<pre class="sourceCode ruby"><code class="sourceCode ruby"><span class="co"># app/models/survey.rb</span>
<span class="kw">def</span> summarize(summarizer)
questions.map <span class="kw">do</span> |question|
question.summarize(summarizer)
<span class="kw">end</span>
<span class="kw">end</span></code></pre>
<p>The <code>summarize</code> method on <code>Question</code> gets a value by calling <code>summarize</code> on a summarizer, and then builds a <code>Summary</code> using that value.</p>
<pre class="sourceCode ruby"><code class="sourceCode ruby"><span class="co"># app/models/question.rb</span>
<span class="kw">def</span> summarize(summarizer)
value = summarizer.summarize(<span class="dv">self</span>)
<span class="dt">Summary</span>.new(title, value)
<span class="kw">end</span></code></pre>
<p>There are several summarizer classes, each of which respond to <code>summarize</code>.</p>
<p>If you're lost, don't worry: You're not the only one. The confusing maze of similar names makes this example extremely hard to follow.</p>
<p>See <a href="#rename-method">rename method</a> to see how we improve the situation.</p>
</section>
<section id="solutions-8" class="level3">
<h3><a href="#solutions-8">Solutions</a></h3>
<ul>
<li><a href="#rename-method">Rename method</a> if a well-factored method isn't well named.</li>
<li><a href="#extract-class">Extract class</a> if a class is doing too much to have a meaningful name.</li>
<li><a href="#extract-method">Extract method</a> if a method is doing too much to have a meaningful name.</li>
<li><a href="#inline-class">Inline class</a> if a class is too abstract to have a meaningful name.</li>
</ul>
</section>
</section>
<section id="single-table-inheritance-sti" class="level1">
<h1><a href="#single-table-inheritance-sti">Single Table Inheritance (STI)</a></h1>
<p>Using subclasses is a common method of achieving reuse in object-oriented software. Rails provides a mechanism for storing instances of different classes in the same table, called Single Table Inheritance. Rails takes care of most of the details by writing the class's name to the type column and instantiating the correct class when results come back from the database.</p>
<p>Inheritance has its own pitfalls (see <a href="#composition-over-inheritance">composition over inheritance</a>) and STI introduces a few new gotchas that may compel you to consider an alternate solution.</p>
<section id="symptoms-9" class="level3">
<h3><a href="#symptoms-9">Symptoms</a></h3>
<ul>
<li>You need to change from one subclass to another.</li>
<li>Behavior is shared among some subclasses but not others.</li>
<li>One subclass is a fusion of one or more other subclasses.</li>
</ul>
<p></p>
</section>
<section id="example-9" class="level3">
<h3><a href="#example-9">Example</a></h3>
<p>This method on <code>Question</code> changes the question to a new type. Any necessary attributes for the new subclass are provided to the <code>attributes</code> method.</p>
<pre class="sourceCode ruby"><code class="sourceCode ruby"><span class="co"># app/models/question.rb</span>
<span class="kw">def</span> switch_to(type, new_attributes)
attributes = <span class="dv">self</span>.attributes.merge(new_attributes)
new_question = type.constantize.new(attributes.except(<span class="st">'id'</span>, <span class="st">'type'</span>))
new_question.id = id
<span class="kw">begin</span>
<span class="dt">Question</span>.transaction <span class="kw">do</span>
destroy
new_question.save!
<span class="kw">end</span>
<span class="kw">rescue</span> <span class="dt">ActiveRecord</span>::<span class="dt">RecordInvalid</span>
<span class="kw">end</span>
new_question
<span class="kw">end</span></code></pre>
<p>This transition is difficult for a number of reasons:</p>
<ul>
<li>You need to worry about common <code>Question</code> validations.</li>
<li>You need to make sure validations for the old subclass are not used.</li>
<li>You need to make sure validations for the new subclass are used.</li>
<li>You need to delete data from the old subclass, including associations.</li>
<li>You need to support data from the new subclass.</li>
<li>Common attributes need to remain the same.</li>
</ul>
<p>The implementation achieves all these requirements, but is awkward:</p>
<ul>
<li>You can't actually change the class of an instance in Ruby, so you need to return the instance of the new class.</li>
<li>The implementation requires deleting and creating records, but part of the transaction (<code>destroy</code>) must execute before you can validate the new instance. This results in control flow using exceptions.</li>
<li>The STI abstraction leaks into the model, because it needs to understand that it has a <code>type</code> column. STI models normally don't need to understand that they're implemented using STI.</li>
<li>It's hard to understand why this method is implemented the way it is, so other developers fixing bugs or refactoring in the future will have a hard time navigating it.</li>
</ul>
</section>
<section id="solutions-9" class="level3">
<h3><a href="#solutions-9">Solutions</a></h3>
<ul>
<li>If you're using STI to reuse common behavior, use <a href="#replace-subclasses-with-strategies">replace subclasses with strategies</a> to switch to a composition-based model.</li>
<li>If you're using STI so that you can easily refer to several different classes in the same table, switch to using a polymorphic association instead.</li>
</ul>
</section>
<section id="prevention-5" class="level3">
<h3><a href="#prevention-5">Prevention</a></h3>
<p>By following <a href="#composition-over-inheritance">composition over inheritance</a>, you'll use STI as a solution less often.</p>
</section>
</section>
<section id="comments" class="level1">
<h1><a href="#comments">Comments</a></h1>
<p>Comments can be used appropriately to introduce classes and provide documentation. But used incorrectly, they mask readability and process problems by further obfuscating already unreadable code.</p>
<section id="symptoms-10" class="level3">
<h3><a href="#symptoms-10">Symptoms</a></h3>
<ul>
<li>Comments within method bodies.</li>
<li>More than one comment per method.</li>
<li>Comments that restate the method name in English.</li>
<li>Todo comments.</li>
<li>Commented out, dead code.</li>
</ul>
</section>
<section id="example-10" class="level3">
<h3><a href="#example-10">Example</a></h3>
<pre class="sourceCode ruby"><code class="sourceCode ruby"><span class="co"># app/models/open_question.rb</span>
<span class="kw">def</span> summary