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 PhpSecInfo_Test 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 PhpSecInfo_Test, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
36 | class PhpSecInfo_Test |
||
37 | { |
||
38 | |||
39 | /** |
||
40 | * This value is used to group test results together. |
||
41 | * |
||
42 | * For example, all tests related to the mysql lib should be grouped under "mysql." |
||
43 | * |
||
44 | * @var string |
||
45 | */ |
||
46 | var $test_group = 'misc'; |
||
47 | |||
48 | |||
49 | /** |
||
50 | * This should be a <b>unique</b>, human-readable identifier for this test |
||
51 | * |
||
52 | * @var string |
||
53 | */ |
||
54 | var $test_name = 'misc_test'; |
||
55 | |||
56 | |||
57 | /** |
||
58 | * This is the recommended value the test will be looking for |
||
59 | * |
||
60 | * @var mixed |
||
61 | */ |
||
62 | var $recommended_value = "bar"; |
||
63 | |||
64 | |||
65 | /** |
||
66 | * The result returned from the test |
||
67 | * |
||
68 | * @var integer |
||
69 | */ |
||
70 | var $_result = PHPSECINFO_TEST_RESULT_NOTRUN; |
||
71 | |||
72 | |||
73 | /** |
||
74 | * The message corresponding to the result of the test |
||
75 | * |
||
76 | * @var string |
||
77 | */ |
||
78 | var $_message; |
||
79 | |||
80 | |||
81 | /** |
||
82 | * the language code. Should be a pointer to the setting in the PhpSecInfo object |
||
83 | * |
||
84 | * @var string |
||
85 | */ |
||
86 | var $_language = PHPSECINFO_LANG_DEFAULT; |
||
87 | |||
88 | /** |
||
89 | * Enter description here... |
||
90 | * |
||
91 | * @var mixed |
||
92 | */ |
||
93 | var $current_value; |
||
94 | |||
95 | /** |
||
96 | * This is a hash of messages that correspond to various test result levels. |
||
97 | * |
||
98 | * There are five messages, each corresponding to one of the result constants |
||
99 | * (PHPSECINFO_TEST_RESULT_OK, PHPSECINFO_TEST_RESULT_NOTICE, PHPSECINFO_TEST_RESULT_WARN, |
||
100 | * PHPSECINFO_TEST_RESULT_ERROR, PHPSECINFO_TEST_RESULT_NOTRUN) |
||
101 | * |
||
102 | * |
||
103 | * @var array array |
||
104 | */ |
||
105 | var $_messages = array(); |
||
106 | |||
107 | |||
108 | |||
109 | |||
110 | /** |
||
111 | * Constructor for Test skeleton class |
||
112 | * |
||
113 | * @return PhpSecInfo_Test |
||
114 | */ |
||
115 | function __construct() { |
||
116 | //$this->_setTestValues(); |
||
117 | |||
118 | $this->_retrieveCurrentValue(); |
||
119 | //$this->setRecommendedValue(); |
||
120 | |||
121 | $this->_setMessages(); |
||
122 | } |
||
123 | |||
124 | |||
125 | /** |
||
126 | * Determines whether or not it's appropriate to run this test (for example, if |
||
127 | * this test is for a particular library, it shouldn't be run if the lib isn't |
||
128 | * loaded). |
||
129 | * |
||
130 | * This is a terrible name, but I couldn't think of a better one atm. |
||
131 | * |
||
132 | * @return boolean |
||
133 | */ |
||
134 | function isTestable() { |
||
138 | |||
139 | |||
140 | /** |
||
141 | * The "meat" of the test. This is where the real test code goes. You should override this when extending |
||
142 | * |
||
143 | * @return integer |
||
144 | */ |
||
145 | function _execTest() { |
||
149 | |||
150 | |||
151 | /** |
||
152 | * This function loads up result messages into the $this->_messages array. |
||
153 | * |
||
154 | * Using this method rather than setting $this->_messages directly allows result |
||
155 | * messages to be inherited. This is broken out into a separate function rather |
||
156 | * than the constructor for ease of extension purposes (php4 is whack, man). |
||
157 | * |
||
158 | */ |
||
159 | View Code Duplication | function _setMessages() { |
|
166 | |||
167 | |||
168 | /** |
||
169 | * Placeholder - extend for tests |
||
170 | * |
||
171 | */ |
||
172 | function _retrieveCurrentValue() { |
||
175 | |||
176 | |||
177 | |||
178 | /** |
||
179 | * This is the wrapper that executes the test and sets the result code and message |
||
180 | */ |
||
181 | function test() { |
||
186 | |||
187 | |||
188 | |||
189 | /** |
||
190 | * Retrieves the result |
||
191 | * |
||
192 | * @return integer |
||
193 | */ |
||
194 | function getResult() { |
||
197 | |||
198 | |||
199 | |||
200 | |||
201 | /** |
||
202 | * Retrieves the message for the current result |
||
203 | * |
||
204 | * @return string |
||
205 | */ |
||
206 | function getMessage() { |
||
213 | |||
214 | |||
215 | |||
216 | /** |
||
217 | * Sets the message for a given result code and language |
||
218 | * |
||
219 | * <code> |
||
220 | * $this->setMessageForResult(PHPSECINFO_TEST_RESULT_NOTRUN, 'en', 'This test cannot be run'); |
||
221 | * </code> |
||
222 | * |
||
223 | * @param integer $result_code |
||
224 | * @param string $language_code |
||
225 | * @param string $message |
||
226 | * |
||
227 | */ |
||
228 | function setMessageForResult($result_code, $language_code, $message) { |
||
241 | |||
242 | |||
243 | |||
244 | |||
245 | /** |
||
246 | * returns the current value. This function should be used to access the |
||
247 | * value for display. All values are cast as strings |
||
248 | * |
||
249 | * @return string |
||
250 | */ |
||
251 | function getCurrentTestValue() { |
||
254 | |||
255 | /** |
||
256 | * returns the recommended value. This function should be used to access the |
||
257 | * value for display. All values are cast as strings |
||
258 | * |
||
259 | * @return string |
||
260 | */ |
||
261 | function getRecommendedTestValue() { |
||
264 | |||
265 | |||
266 | /** |
||
267 | * Sets the result code |
||
268 | * |
||
269 | * @param integer $result_code |
||
270 | */ |
||
271 | function _setResult($result_code) { |
||
274 | |||
275 | |||
276 | /** |
||
277 | * Sets the $this->_message variable based on the passed result and language codes |
||
278 | * |
||
279 | * @param integer $result_code |
||
280 | * @param string $language_code |
||
281 | */ |
||
282 | function _setMessage($result_code, $language_code) { |
||
287 | |||
288 | |||
289 | /** |
||
290 | * Returns a link to a page with detailed information about the test |
||
291 | * |
||
292 | * URL is formatted as PHPSECINFO_TEST_MOREINFO_BASEURL + testName |
||
293 | * |
||
294 | * @see PHPSECINFO_TEST_MOREINFO_BASEURL |
||
295 | * |
||
296 | * @return string|boolean |
||
297 | */ |
||
298 | function getMoreInfoURL() { |
||
305 | |||
306 | |||
307 | |||
308 | |||
309 | /** |
||
310 | * This retrieves the name of this test. |
||
311 | * |
||
312 | * If a name has not been set, this returns a formatted version of the class name. |
||
313 | * |
||
314 | * @return string |
||
315 | */ |
||
316 | function getTestName() { |
||
328 | |||
329 | |||
330 | /** |
||
331 | * sets the test name |
||
332 | * |
||
333 | * @param string $test_name |
||
334 | */ |
||
335 | function setTestName($test_name) { |
||
338 | |||
339 | |||
340 | /** |
||
341 | * Returns the test group this test belongs to |
||
342 | * |
||
343 | * @return string |
||
344 | */ |
||
345 | function getTestGroup() { |
||
348 | |||
349 | |||
350 | /** |
||
351 | * sets the test group |
||
352 | * |
||
353 | * @param string $test_group |
||
354 | */ |
||
355 | function setTestGroup($test_group) { |
||
358 | |||
359 | |||
360 | /** |
||
361 | * This function takes the shorthand notation used in memory limit settings for PHP |
||
362 | * and returns the byte value. Totally stolen from http://us3.php.net/manual/en/function.ini-get.php |
||
363 | * |
||
364 | * <code> |
||
365 | * echo 'post_max_size in bytes = ' . $this->return_bytes(ini_get('post_max_size')); |
||
366 | * </code> |
||
367 | * |
||
368 | * @link http://php.net/manual/en/function.ini-get.php |
||
369 | * @param string $val |
||
370 | * @return integer |
||
371 | */ |
||
372 | function returnBytes($val) { |
||
392 | |||
393 | |||
394 | /** |
||
395 | * This just does the usual PHP string casting, except for |
||
396 | * the boolean FALSE value, where the string "0" is returned |
||
397 | * instead of an empty string |
||
398 | * |
||
399 | * @param mixed $val |
||
400 | * @return string |
||
401 | */ |
||
402 | function getStringValue($val) { |
||
409 | |||
410 | |||
411 | /** |
||
412 | * This method converts the several possible return values from |
||
413 | * allegedly "boolean" ini settings to proper booleans |
||
414 | * |
||
415 | * Properly converted input values are: 'off', 'on', 'false', 'true', '', '0', '1' |
||
416 | * (the last two might not be neccessary, but I'd rather be safe) |
||
417 | * |
||
418 | * If the ini_value doesn't match any of those, the value is returned as-is. |
||
419 | * |
||
420 | * @param string $ini_key the ini_key you need the value of |
||
421 | * @return boolean|mixed |
||
422 | */ |
||
423 | function getBooleanIniValue($ini_key) { |
||
456 | |||
457 | /** |
||
458 | * sys_get_temp_dir provides some temp dir detection capability |
||
459 | * that is lacking in versions of PHP that do not have the |
||
460 | * sys_get_temp_dir() function |
||
461 | * |
||
462 | * @return string|NULL |
||
463 | */ |
||
464 | function sys_get_temp_dir() { |
||
475 | |||
476 | |||
477 | /** |
||
478 | * A quick function to determine whether we're running on Windows. |
||
479 | * Uses the PHP_OS constant. |
||
480 | * |
||
481 | * @return boolean |
||
482 | */ |
||
483 | function osIsWindows() { |
||
490 | |||
491 | |||
492 | /** |
||
493 | * Returns an array of data returned from the UNIX 'id' command |
||
494 | * |
||
495 | * includes uid, username, gid, groupname, and groups (if "exec" |
||
496 | * is enabled). Groups is an array of all the groups the user |
||
497 | * belongs to. Keys are the group ids, values are the group names. |
||
498 | * |
||
499 | * returns FALSE if no suitable function is available to retrieve |
||
500 | * the data |
||
501 | * |
||
502 | * @return array|boolean |
||
503 | */ |
||
504 | function getUnixId() { |
||
574 | |||
575 | } |
||
576 |
The PSR-1: Basic Coding Standard recommends that a file should either introduce new symbols, that is classes, functions, constants or similar, or have side effects. Side effects are anything that executes logic, like for example printing output, changing ini settings or writing to a file.
The idea behind this recommendation is that merely auto-loading a class should not change the state of an application. It also promotes a cleaner style of programming and makes your code less prone to errors, because the logic is not spread out all over the place.
To learn more about the PSR-1, please see the PHP-FIG site on the PSR-1.