wikimedia /
mediawiki
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 ApiQueryAllMessages 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 ApiQueryAllMessages, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 32 | class ApiQueryAllMessages extends ApiQueryBase { |
||
| 33 | |||
| 34 | public function __construct( ApiQuery $query, $moduleName ) { |
||
| 37 | |||
| 38 | public function execute() { |
||
| 39 | $params = $this->extractRequestParams(); |
||
| 40 | |||
| 41 | if ( is_null( $params['lang'] ) ) { |
||
| 42 | $langObj = $this->getLanguage(); |
||
| 43 | } elseif ( !Language::isValidCode( $params['lang'] ) ) { |
||
| 44 | $this->dieUsage( 'Invalid language code for parameter lang', 'invalidlang' ); |
||
| 45 | } else { |
||
| 46 | $langObj = Language::factory( $params['lang'] ); |
||
| 47 | } |
||
| 48 | |||
| 49 | if ( $params['enableparser'] ) { |
||
| 50 | if ( !is_null( $params['title'] ) ) { |
||
| 51 | $title = Title::newFromText( $params['title'] ); |
||
| 52 | if ( !$title || $title->isExternal() ) { |
||
| 53 | $this->dieUsageMsg( [ 'invalidtitle', $params['title'] ] ); |
||
| 54 | } |
||
| 55 | } else { |
||
| 56 | $title = Title::newFromText( 'API' ); |
||
| 57 | } |
||
| 58 | } |
||
| 59 | |||
| 60 | $prop = array_flip( (array)$params['prop'] ); |
||
| 61 | |||
| 62 | // Determine which messages should we print |
||
| 63 | if ( in_array( '*', $params['messages'] ) ) { |
||
| 64 | $message_names = Language::getMessageKeysFor( $langObj->getCode() ); |
||
|
|
|||
| 65 | if ( $params['includelocal'] ) { |
||
| 66 | $message_names = array_unique( array_merge( |
||
| 67 | $message_names, |
||
| 68 | // Pass in the content language code so we get local messages that have a |
||
| 69 | // MediaWiki:msgkey page. We might theoretically miss messages that have no |
||
| 70 | // MediaWiki:msgkey page but do have a MediaWiki:msgkey/lang page, but that's |
||
| 71 | // just a stupid case. |
||
| 72 | MessageCache::singleton()->getAllMessageKeys( $this->getConfig()->get( 'LanguageCode' ) ) |
||
| 73 | ) ); |
||
| 74 | } |
||
| 75 | sort( $message_names ); |
||
| 76 | $messages_target = $message_names; |
||
| 77 | } else { |
||
| 78 | $messages_target = $params['messages']; |
||
| 79 | } |
||
| 80 | |||
| 81 | // Filter messages that have the specified prefix |
||
| 82 | // Because we sorted the message array earlier, they will appear in a clump: |
||
| 83 | if ( isset( $params['prefix'] ) ) { |
||
| 84 | $skip = false; |
||
| 85 | $messages_filtered = []; |
||
| 86 | foreach ( $messages_target as $message ) { |
||
| 87 | // === 0: must be at beginning of string (position 0) |
||
| 88 | if ( strpos( $message, $params['prefix'] ) === 0 ) { |
||
| 89 | if ( !$skip ) { |
||
| 90 | $skip = true; |
||
| 91 | } |
||
| 92 | $messages_filtered[] = $message; |
||
| 93 | } elseif ( $skip ) { |
||
| 94 | break; |
||
| 95 | } |
||
| 96 | } |
||
| 97 | $messages_target = $messages_filtered; |
||
| 98 | } |
||
| 99 | |||
| 100 | // Filter messages that contain specified string |
||
| 101 | if ( isset( $params['filter'] ) ) { |
||
| 102 | $messages_filtered = []; |
||
| 103 | foreach ( $messages_target as $message ) { |
||
| 104 | // !== is used because filter can be at the beginning of the string |
||
| 105 | if ( strpos( $message, $params['filter'] ) !== false ) { |
||
| 106 | $messages_filtered[] = $message; |
||
| 107 | } |
||
| 108 | } |
||
| 109 | $messages_target = $messages_filtered; |
||
| 110 | } |
||
| 111 | |||
| 112 | // Whether we have any sort of message customisation filtering |
||
| 113 | $customiseFilterEnabled = $params['customised'] !== 'all'; |
||
| 114 | if ( $customiseFilterEnabled ) { |
||
| 115 | global $wgContLang; |
||
| 116 | |||
| 117 | $customisedMessages = AllMessagesTablePager::getCustomisedStatuses( |
||
| 118 | array_map( |
||
| 119 | [ $langObj, 'ucfirst' ], |
||
| 120 | $messages_target |
||
| 121 | ), |
||
| 122 | $langObj->getCode(), |
||
| 123 | !$langObj->equals( $wgContLang ) |
||
| 124 | ); |
||
| 125 | |||
| 126 | $customised = $params['customised'] === 'modified'; |
||
| 127 | } |
||
| 128 | |||
| 129 | // Get all requested messages and print the result |
||
| 130 | $skip = !is_null( $params['from'] ); |
||
| 131 | $useto = !is_null( $params['to'] ); |
||
| 132 | $result = $this->getResult(); |
||
| 133 | foreach ( $messages_target as $message ) { |
||
| 134 | // Skip all messages up to $params['from'] |
||
| 135 | if ( $skip && $message === $params['from'] ) { |
||
| 136 | $skip = false; |
||
| 137 | } |
||
| 138 | |||
| 139 | if ( $useto && $message > $params['to'] ) { |
||
| 140 | break; |
||
| 141 | } |
||
| 142 | |||
| 143 | if ( !$skip ) { |
||
| 144 | $a = [ |
||
| 145 | 'name' => $message, |
||
| 146 | 'normalizedname' => MessageCache::normalizeKey( $message ), |
||
| 147 | ]; |
||
| 148 | |||
| 149 | $args = []; |
||
| 150 | View Code Duplication | if ( isset( $params['args'] ) && count( $params['args'] ) != 0 ) { |
|
| 151 | $args = $params['args']; |
||
| 152 | } |
||
| 153 | |||
| 154 | if ( $customiseFilterEnabled ) { |
||
| 155 | $messageIsCustomised = isset( $customisedMessages['pages'][$langObj->ucfirst( $message )] ); |
||
| 156 | if ( $customised === $messageIsCustomised ) { |
||
| 157 | if ( $customised ) { |
||
| 158 | $a['customised'] = true; |
||
| 159 | } |
||
| 160 | } else { |
||
| 161 | continue; |
||
| 162 | } |
||
| 163 | } |
||
| 164 | |||
| 165 | $msg = wfMessage( $message, $args )->inLanguage( $langObj ); |
||
| 166 | |||
| 167 | if ( !$msg->exists() ) { |
||
| 168 | $a['missing'] = true; |
||
| 169 | } else { |
||
| 170 | // Check if the parser is enabled: |
||
| 171 | if ( $params['enableparser'] ) { |
||
| 172 | $msgString = $msg->title( $title )->text(); |
||
| 173 | } else { |
||
| 174 | $msgString = $msg->plain(); |
||
| 175 | } |
||
| 176 | if ( !$params['nocontent'] ) { |
||
| 177 | ApiResult::setContentValue( $a, 'content', $msgString ); |
||
| 178 | } |
||
| 179 | if ( isset( $prop['default'] ) ) { |
||
| 180 | $default = wfMessage( $message )->inLanguage( $langObj )->useDatabase( false ); |
||
| 181 | if ( !$default->exists() ) { |
||
| 182 | $a['defaultmissing'] = true; |
||
| 183 | } elseif ( $default->plain() != $msgString ) { |
||
| 184 | $a['default'] = $default->plain(); |
||
| 185 | } |
||
| 186 | } |
||
| 187 | } |
||
| 188 | $fit = $result->addValue( [ 'query', $this->getModuleName() ], null, $a ); |
||
| 189 | if ( !$fit ) { |
||
| 190 | $this->setContinueEnumParameter( 'from', $message ); |
||
| 191 | break; |
||
| 192 | } |
||
| 193 | } |
||
| 194 | } |
||
| 195 | $result->addIndexedTagName( [ 'query', $this->getModuleName() ], 'message' ); |
||
| 196 | } |
||
| 197 | |||
| 198 | public function getCacheMode( $params ) { |
||
| 210 | |||
| 211 | public function getAllowedParams() { |
||
| 246 | |||
| 247 | protected function getExamplesMessages() { |
||
| 255 | |||
| 256 | public function getHelpUrls() { |
||
| 259 | } |
||
| 260 |
Loading history...
If you define a variable conditionally, it can happen that it is not defined for all execution paths.
Let’s take a look at an example:
In the above example, the variable $x is defined if you pass “foo” or “bar” as argument for $a. However, since the switch statement has no default case statement, if you pass any other value, the variable $x would be undefined.
Available Fixes
Check for existence of the variable explicitly:
Define a default value for the variable:
Add a value for the missing path: