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:
Complex classes like LBFactoryMulti often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.
Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.
While breaking up the class, it is a good idea to analyze how other classes use LBFactoryMulti, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
30 | class LBFactoryMulti extends LBFactory { |
||
31 | /** @var array A map of database names to section names */ |
||
32 | private $sectionsByDB; |
||
33 | |||
34 | /** |
||
35 | * @var array A 2-d map. For each section, gives a map of server names to |
||
36 | * load ratios |
||
37 | */ |
||
38 | private $sectionLoads; |
||
39 | |||
40 | /** |
||
41 | * @var array[] Server info associative array |
||
42 | * @note The host, hostName and load entries will be overridden |
||
43 | */ |
||
44 | private $serverTemplate; |
||
45 | |||
46 | // Optional settings |
||
47 | |||
48 | /** @var array A 3-d map giving server load ratios for each section and group */ |
||
49 | private $groupLoadsBySection = []; |
||
50 | |||
51 | /** @var array A 3-d map giving server load ratios by DB name */ |
||
52 | private $groupLoadsByDB = []; |
||
53 | |||
54 | /** @var array A map of hostname to IP address */ |
||
55 | private $hostsByName = []; |
||
56 | |||
57 | /** @var array A map of external storage cluster name to server load map */ |
||
58 | private $externalLoads = []; |
||
59 | |||
60 | /** |
||
61 | * @var array A set of server info keys overriding serverTemplate for |
||
62 | * external storage |
||
63 | */ |
||
64 | private $externalTemplateOverrides; |
||
65 | |||
66 | /** |
||
67 | * @var array A 2-d map overriding serverTemplate and |
||
68 | * externalTemplateOverrides on a server-by-server basis. Applies to both |
||
69 | * core and external storage |
||
70 | */ |
||
71 | private $templateOverridesByServer; |
||
72 | |||
73 | /** @var array A 2-d map overriding the server info by section */ |
||
74 | private $templateOverridesBySection; |
||
75 | |||
76 | /** @var array A 2-d map overriding the server info by external storage cluster */ |
||
77 | private $templateOverridesByCluster; |
||
78 | |||
79 | /** @var array An override array for all master servers */ |
||
80 | private $masterTemplateOverrides; |
||
81 | |||
82 | /** |
||
83 | * @var array|bool A map of section name to read-only message. Missing or |
||
84 | * false for read/write |
||
85 | */ |
||
86 | private $readOnlyBySection = []; |
||
87 | |||
88 | /** @var array Load balancer factory configuration */ |
||
89 | private $conf; |
||
90 | |||
91 | /** @var LoadBalancer[] */ |
||
92 | private $mainLBs = []; |
||
93 | |||
94 | /** @var LoadBalancer[] */ |
||
95 | private $extLBs = []; |
||
96 | |||
97 | /** @var string */ |
||
98 | private $loadMonitorClass = 'LoadMonitor'; |
||
99 | |||
100 | /** @var string */ |
||
101 | private $lastDomain; |
||
102 | |||
103 | /** @var string */ |
||
104 | private $lastSection; |
||
105 | |||
106 | /** |
||
107 | * @see LBFactory::__construct() |
||
108 | * |
||
109 | * Template override precedence (highest => lowest): |
||
110 | * - templateOverridesByServer |
||
111 | * - masterTemplateOverrides |
||
112 | * - templateOverridesBySection/templateOverridesByCluster |
||
113 | * - externalTemplateOverrides |
||
114 | * - serverTemplate |
||
115 | * Overrides only work on top level keys (so nested values will not be merged). |
||
116 | * |
||
117 | * Server configuration maps should be of the format Database::factory() requires. |
||
118 | * Additionally, a 'max lag' key should also be set on server maps, indicating how stale the |
||
119 | * data can be before the load balancer tries to avoid using it. The map can have 'is static' |
||
120 | * set to disable blocking replication sync checks (intended for archive servers with |
||
121 | * unchanging data). |
||
122 | * |
||
123 | * @param array $conf Parameters of LBFactory::__construct() as well as: |
||
124 | * - sectionsByDB Map of database names to section names. |
||
125 | * - sectionLoads 2-d map. For each section, gives a map of server names to |
||
126 | * load ratios. For example: |
||
127 | * [ |
||
128 | * 'section1' => [ |
||
129 | * 'db1' => 100, |
||
130 | * 'db2' => 100 |
||
131 | * ] |
||
132 | * ] |
||
133 | * - serverTemplate Server configuration map intended for Database::factory(). |
||
134 | * Note that "host", "hostName" and "load" entries will be |
||
135 | * overridden by "sectionLoads" and "hostsByName". |
||
136 | * - groupLoadsBySection 3-d map giving server load ratios for each section/group. |
||
137 | * For example: |
||
138 | * [ |
||
139 | * 'section1' => [ |
||
140 | * 'group1' => [ |
||
141 | * 'db1' => 100, |
||
142 | * 'db2' => 100 |
||
143 | * ] |
||
144 | * ] |
||
145 | * ] |
||
146 | * - groupLoadsByDB 3-d map giving server load ratios by DB name. |
||
147 | * - hostsByName Map of hostname to IP address. |
||
148 | * - externalLoads Map of external storage cluster name to server load map. |
||
149 | * - externalTemplateOverrides Set of server configuration maps overriding |
||
150 | * "serverTemplate" for external storage. |
||
151 | * - templateOverridesByServer 2-d map overriding "serverTemplate" and |
||
152 | * "externalTemplateOverrides" on a server-by-server basis. |
||
153 | * Applies to both core and external storage. |
||
154 | * - templateOverridesBySection 2-d map overriding the server configuration maps by section. |
||
155 | * - templateOverridesByCluster 2-d map overriding the server configuration maps by external |
||
156 | * storage cluster. |
||
157 | * - masterTemplateOverrides Server configuration map overrides for all master servers. |
||
158 | * - loadMonitorClass Name of the LoadMonitor class to always use. |
||
159 | * - readOnlyBySection A map of section name to read-only message. |
||
160 | * Missing or false for read/write. |
||
161 | */ |
||
162 | public function __construct( array $conf ) { |
||
163 | parent::__construct( $conf ); |
||
164 | |||
165 | $this->conf = $conf; |
||
166 | $required = [ 'sectionsByDB', 'sectionLoads', 'serverTemplate' ]; |
||
167 | $optional = [ 'groupLoadsBySection', 'groupLoadsByDB', 'hostsByName', |
||
168 | 'externalLoads', 'externalTemplateOverrides', 'templateOverridesByServer', |
||
169 | 'templateOverridesByCluster', 'templateOverridesBySection', 'masterTemplateOverrides', |
||
170 | 'readOnlyBySection', 'loadMonitorClass' ]; |
||
171 | |||
172 | View Code Duplication | foreach ( $required as $key ) { |
|
173 | if ( !isset( $conf[$key] ) ) { |
||
174 | throw new InvalidArgumentException( __CLASS__ . ": $key is required." ); |
||
175 | } |
||
176 | $this->$key = $conf[$key]; |
||
177 | } |
||
178 | |||
179 | foreach ( $optional as $key ) { |
||
180 | if ( isset( $conf[$key] ) ) { |
||
181 | $this->$key = $conf[$key]; |
||
182 | } |
||
183 | } |
||
184 | } |
||
185 | |||
186 | /** |
||
187 | * @param bool|string $domain |
||
188 | * @return string |
||
189 | */ |
||
190 | private function getSectionForDomain( $domain = false ) { |
||
205 | |||
206 | /** |
||
207 | * @param bool|string $domain |
||
208 | * @return LoadBalancer |
||
209 | */ |
||
210 | public function newMainLB( $domain = false ) { |
||
242 | |||
243 | /** |
||
244 | * @param DatabaseDomain|string|bool $domain Domain ID, or false for the current domain |
||
245 | * @return LoadBalancer |
||
246 | */ |
||
247 | public function getMainLB( $domain = false ) { |
||
257 | |||
258 | public function newExternalLB( $cluster ) { |
||
277 | |||
278 | View Code Duplication | public function getExternalLB( $cluster ) { |
|
286 | |||
287 | public function getAllMainLBs() { |
||
297 | |||
298 | public function getAllExternalLBs() { |
||
306 | |||
307 | /** |
||
308 | * Make a new load balancer object based on template and load array |
||
309 | * |
||
310 | * @param array $template |
||
311 | * @param array $loads |
||
312 | * @param array $groupLoads |
||
313 | * @param string|bool $readOnlyReason |
||
314 | * @return LoadBalancer |
||
315 | */ |
||
316 | private function newLoadBalancer( $template, $loads, $groupLoads, $readOnlyReason ) { |
||
329 | |||
330 | /** |
||
331 | * Make a server array as expected by LoadBalancer::__construct, using a template and load array |
||
332 | * |
||
333 | * @param array $template |
||
334 | * @param array $loads |
||
335 | * @param array $groupLoads |
||
336 | * @return array |
||
337 | */ |
||
338 | private function makeServerArray( $template, $loads, $groupLoads ) { |
||
378 | |||
379 | /** |
||
380 | * Take a group load array indexed by group then server, and reindex it by server then group |
||
381 | * @param array $groupLoads |
||
382 | * @return array |
||
383 | */ |
||
384 | private function reindexGroupLoads( $groupLoads ) { |
||
394 | |||
395 | /** |
||
396 | * @param DatabaseDomain|string|bool $domain Domain ID, or false for the current domain |
||
397 | * @return array [database name, table prefix] |
||
398 | */ |
||
399 | private function getDBNameAndPrefix( $domain = false ) { |
||
406 | |||
407 | /** |
||
408 | * Execute a function for each tracked load balancer |
||
409 | * The callback is called with the load balancer as the first parameter, |
||
410 | * and $params passed as the subsequent parameters. |
||
411 | * @param callable $callback |
||
412 | * @param array $params |
||
413 | */ |
||
414 | public function forEachLB( $callback, array $params = [] ) { |
||
422 | } |
||
423 |
Our type inference engine has found a suspicous assignment of a value to a property. This check raises an issue when a value that can be of a mixed type is assigned to a property that is type hinted more strictly.
For example, imagine you have a variable
$accountId
that can either hold an Id object or false (if there is no account id yet). Your code now assigns that value to theid
property of an instance of theAccount
class. This class holds a proper account, so the id value must no longer be false.Either this assignment is in error or a type check should be added for that assignment.