Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.
Common duplication problems, and corresponding solutions are:
1 | <?php |
||
25 | class Configuration implements ConfigurationInterface |
||
26 | { |
||
27 | /** |
||
28 | * {@inheritdoc} |
||
29 | */ |
||
30 | 12 | public function getConfigTreeBuilder() |
|
31 | { |
||
32 | 12 | $treeBuilder = new TreeBuilder(); |
|
33 | |||
34 | 12 | $rootNode = $treeBuilder->root('runopencode_exchange_rate'); |
|
35 | |||
36 | $rootNode |
||
37 | 12 | ->fixXmlConfig('source', 'sources') |
|
38 | 12 | ->children() |
|
39 | 12 | ->scalarNode('base_currency') |
|
40 | 12 | ->isRequired() |
|
41 | 12 | ->info('Set base currency in which you are doing your business activities.') |
|
42 | 12 | ->end() |
|
43 | 12 | ->scalarNode('repository') |
|
44 | 12 | ->defaultValue('file') |
|
45 | 12 | ->info('Service ID which is in charge for rates persistence.') |
|
46 | 12 | ->end() |
|
47 | 12 | ->append($this->getRatesDefinition()) |
|
48 | 12 | ->append($this->getFileRepositoryDefinition()) |
|
49 | 12 | ->append($this->getDoctrineDbalRepositoryDefinition()) |
|
50 | 12 | ->append($this->getSourcesDefinition()) |
|
51 | 12 | ->append($this->getAccessRolesDefinition()) |
|
52 | 12 | ->arrayNode('form_types') |
|
53 | 12 | ->addDefaultsIfNotSet() |
|
54 | 12 | ->children() |
|
55 | 12 | ->append($this->getSourceTypeDefinition()) |
|
56 | 12 | ->append($this->getRateTypeTypeDefinition()) |
|
57 | 12 | ->append($this->getCurrencyCodeTypeDefinition()) |
|
58 | 12 | ->append($this->getForeignCurrencyCodeTypeDefinition()) |
|
59 | 12 | ->append($this->getRateTypeDefinition()) |
|
60 | 12 | ->end() |
|
61 | 12 | ->end() |
|
62 | 12 | ->append($this->getNotificationsDefinition()) |
|
63 | 12 | ->end() |
|
64 | 12 | ->end(); |
|
65 | |||
66 | 12 | return $treeBuilder; |
|
67 | } |
||
68 | |||
69 | /** |
||
70 | * Build configuration tree for rates. |
||
71 | * |
||
72 | * @return ArrayNodeDefinition |
||
73 | */ |
||
74 | 12 | protected function getRatesDefinition() |
|
75 | { |
||
76 | 12 | $node = new ArrayNodeDefinition('rates'); |
|
77 | |||
78 | $node |
||
79 | 12 | ->beforeNormalization() |
|
80 | 12 | ->ifTrue(function ($value) { |
|
81 | 12 | return array_key_exists('rate', $value); |
|
82 | 12 | }) |
|
83 | 12 | ->then(function ($value) { |
|
84 | 2 | return $value['rate']; |
|
85 | 12 | }) |
|
86 | 12 | ->end(); |
|
87 | |||
88 | $node |
||
|
|||
89 | 12 | ->fixXmlConfig('rate') |
|
90 | 12 | ->info('Configuration of each individual rate with which you intend to work with.') |
|
91 | 12 | ->requiresAtLeastOneElement() |
|
92 | 12 | ->prototype('array') |
|
93 | 12 | ->children() |
|
94 | 12 | ->scalarNode('currency_code')->isRequired()->end() |
|
95 | 12 | ->scalarNode('rate_type')->isRequired()->end() |
|
96 | 12 | ->scalarNode('source')->isRequired()->end() |
|
97 | 12 | ->arrayNode('extra') |
|
98 | 12 | ->useAttributeAsKey('name') |
|
99 | 12 | ->prototype('array') |
|
100 | 12 | ->children() |
|
101 | 12 | ->variableNode('value')->isRequired()->end() |
|
102 | 12 | ->end() |
|
103 | 12 | ->end() |
|
104 | 12 | ->end() |
|
105 | 12 | ->end() |
|
106 | 12 | ->end() |
|
107 | 12 | ->end(); |
|
108 | |||
109 | 12 | return $node; |
|
110 | } |
||
111 | |||
112 | /** |
||
113 | * Build configuration tree for file repository. |
||
114 | * |
||
115 | * @return ArrayNodeDefinition |
||
116 | */ |
||
117 | 12 | protected function getFileRepositoryDefinition() |
|
118 | { |
||
119 | 12 | $node = new ArrayNodeDefinition('file_repository'); |
|
120 | |||
121 | $node |
||
122 | 12 | ->info('Configuration for file repository (if used).') |
|
123 | 12 | ->addDefaultsIfNotSet() |
|
124 | 12 | ->children() |
|
125 | 12 | ->scalarNode('path') |
|
126 | 12 | ->info('Absolute path to file where database file will be stored.') |
|
127 | 12 | ->defaultValue('%kernel.root_dir%/../var/db/exchange_rates.dat') |
|
128 | 12 | ->end() |
|
129 | 12 | ->end() |
|
130 | 12 | ->end(); |
|
131 | |||
132 | 12 | return $node; |
|
133 | } |
||
134 | |||
135 | /** |
||
136 | * Build configuration tree for Doctrine Dbal repository. |
||
137 | * |
||
138 | * @return ArrayNodeDefinition |
||
139 | */ |
||
140 | 12 | protected function getDoctrineDbalRepositoryDefinition() |
|
141 | { |
||
142 | 12 | $node = new ArrayNodeDefinition('doctrine_dbal_repository'); |
|
143 | |||
144 | $node |
||
145 | 12 | ->info('Configuration for Doctrine Dbla repository (if used).') |
|
146 | 12 | ->addDefaultsIfNotSet() |
|
147 | 12 | ->children() |
|
148 | 12 | ->scalarNode('connection') |
|
149 | 12 | ->info('Which database connection to use.') |
|
150 | 12 | ->defaultValue('doctrine.dbal.default_connection') |
|
151 | 12 | ->end() |
|
152 | 12 | ->scalarNode('table_name') |
|
153 | 12 | ->info('Which table name to use for storing exchange rates.') |
|
154 | 12 | ->defaultValue('runopencode_exchange_rate') |
|
155 | 12 | ->end() |
|
156 | 12 | ->end() |
|
157 | 12 | ->end(); |
|
158 | |||
159 | 12 | return $node; |
|
160 | } |
||
161 | |||
162 | /** |
||
163 | * Build configuration tree for simple sources. |
||
164 | * |
||
165 | * @return ArrayNodeDefinition |
||
166 | */ |
||
167 | 12 | protected function getSourcesDefinition() |
|
168 | { |
||
169 | 12 | $node = new ArrayNodeDefinition('sources'); |
|
170 | |||
171 | $node |
||
172 | 12 | ->info('Add sources to sources registry without registering them into service container.') |
|
173 | 12 | ->prototype('scalar')->end() |
|
174 | 12 | ->end(); |
|
175 | |||
176 | 12 | return $node; |
|
177 | } |
||
178 | |||
179 | /** |
||
180 | * Build configuration tree for access roles. |
||
181 | * |
||
182 | * @return ArrayNodeDefinition |
||
183 | */ |
||
184 | 12 | protected function getAccessRolesDefinition() |
|
218 | |||
219 | /** |
||
220 | * Build configuration tree for "RunOpenCode\Bundle\ExchangeRate\Form\Type\SourceType" default settings. |
||
221 | * |
||
222 | * @return ArrayNodeDefinition |
||
223 | */ |
||
224 | 12 | View Code Duplication | protected function getSourceTypeDefinition() |
239 | |||
240 | /** |
||
241 | * Build configuration tree for "RunOpenCode\Bundle\ExchangeRate\Form\Type\RateTypeType" default settings. |
||
242 | * |
||
243 | * @return ArrayNodeDefinition |
||
244 | */ |
||
245 | 12 | View Code Duplication | protected function getRateTypeTypeDefinition() |
260 | |||
261 | /** |
||
262 | * Build configuration tree for "RunOpenCode\Bundle\ExchangeRate\Form\Type\CurrencyCodeType" default settings. |
||
263 | * |
||
264 | * @return ArrayNodeDefinition |
||
265 | */ |
||
266 | 12 | View Code Duplication | protected function getCurrencyCodeTypeDefinition() |
281 | |||
282 | /** |
||
283 | * Build configuration tree for "RunOpenCode\Bundle\ExchangeRate\Form\Type\ForeignCurrencyCodeType" default settings. |
||
284 | * |
||
285 | * @return ArrayNodeDefinition |
||
286 | */ |
||
287 | 12 | View Code Duplication | protected function getForeignCurrencyCodeTypeDefinition() |
302 | |||
303 | /** |
||
304 | * Build configuration tree for "RunOpenCode\Bundle\ExchangeRate\Form\Type\RateType" default settings. |
||
305 | * |
||
306 | * @return ArrayNodeDefinition |
||
307 | */ |
||
308 | 12 | View Code Duplication | protected function getRateTypeDefinition() |
323 | |||
324 | /** |
||
325 | * Build configuration tree for notifications. |
||
326 | * |
||
327 | * @return ArrayNodeDefinition |
||
328 | */ |
||
329 | 12 | protected function getNotificationsDefinition() |
|
372 | } |
||
373 |
Let’s take a look at an example:
In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different sub-classes of User which does not have a getDisplayName() method, the code will break.
Available Fixes
Change the type-hint for the parameter:
Add an additional type-check:
Add the method to the parent class: