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:
| 1 | <?php |
||
| 37 | class RefreshImageMetadata extends Maintenance { |
||
| 38 | |||
| 39 | /** |
||
| 40 | * @var Database |
||
| 41 | */ |
||
| 42 | protected $dbw; |
||
| 43 | |||
| 44 | function __construct() { |
||
| 93 | |||
| 94 | public function execute() { |
||
| 95 | $force = $this->hasOption( 'force' ); |
||
| 96 | $brokenOnly = $this->hasOption( 'broken-only' ); |
||
| 97 | $verbose = $this->hasOption( 'verbose' ); |
||
| 98 | $start = $this->getOption( 'start', false ); |
||
| 99 | $this->setupParameters( $force, $brokenOnly ); |
||
| 100 | |||
| 101 | $upgraded = 0; |
||
| 102 | $leftAlone = 0; |
||
| 103 | $error = 0; |
||
| 104 | |||
| 105 | $dbw = $this->getDB( DB_MASTER ); |
||
| 106 | if ( $this->mBatchSize <= 0 ) { |
||
| 107 | $this->error( "Batch size is too low...", 12 ); |
||
| 108 | } |
||
| 109 | |||
| 110 | $repo = RepoGroup::singleton()->getLocalRepo(); |
||
| 111 | $conds = $this->getConditions( $dbw ); |
||
| 112 | |||
| 113 | // For the WHERE img_name > 'foo' condition that comes after doing a batch |
||
| 114 | $conds2 = []; |
||
| 115 | if ( $start !== false ) { |
||
| 116 | $conds2[] = 'img_name >= ' . $dbw->addQuotes( $start ); |
||
| 117 | } |
||
| 118 | |||
| 119 | $options = [ |
||
| 120 | 'LIMIT' => $this->mBatchSize, |
||
| 121 | 'ORDER BY' => 'img_name ASC', |
||
| 122 | ]; |
||
| 123 | |||
| 124 | do { |
||
| 125 | $res = $dbw->select( |
||
| 126 | 'image', |
||
| 127 | '*', |
||
| 128 | array_merge( $conds, $conds2 ), |
||
| 129 | __METHOD__, |
||
| 130 | $options |
||
| 131 | ); |
||
| 132 | |||
| 133 | if ( $res->numRows() > 0 ) { |
||
| 134 | $row1 = $res->current(); |
||
| 135 | $this->output( "Processing next {$this->mBatchSize} rows starting with {$row1->img_name}.\n" ); |
||
| 136 | $res->rewind(); |
||
| 137 | } else { |
||
| 138 | $this->error( "No images to process.", 4 ); |
||
| 139 | } |
||
| 140 | |||
| 141 | foreach ( $res as $row ) { |
||
| 142 | // LocalFile will upgrade immediately here if obsolete |
||
| 143 | $file = $repo->newFileFromRow( $row ); |
||
| 144 | if ( $file->getUpgraded() ) { |
||
| 145 | // File was upgraded. |
||
| 146 | $upgraded++; |
||
| 147 | $newLength = strlen( $file->getMetadata() ); |
||
| 148 | $oldLength = strlen( $row->img_metadata ); |
||
| 149 | View Code Duplication | if ( $newLength < $oldLength - 5 ) { |
|
| 150 | // If after updating, the metadata is smaller then |
||
| 151 | // what it was before, that's probably not a good thing |
||
| 152 | // because we extract more data with time, not less. |
||
| 153 | // Thus this probably indicates an error of some sort, |
||
| 154 | // or at the very least is suspicious. Have the - 5 just |
||
| 155 | // to weed out any inconsequential changes. |
||
| 156 | $error++; |
||
| 157 | $this->output( "Warning: File:{$row->img_name} used to have " . |
||
| 158 | "$oldLength bytes of metadata but now has $newLength bytes.\n" ); |
||
| 159 | } elseif ( $verbose ) { |
||
| 160 | $this->output( "Refreshed File:{$row->img_name}.\n" ); |
||
| 161 | } |
||
| 162 | } else { |
||
| 163 | $leftAlone++; |
||
| 164 | if ( $force ) { |
||
| 165 | $file->upgradeRow(); |
||
| 166 | $newLength = strlen( $file->getMetadata() ); |
||
| 167 | $oldLength = strlen( $row->img_metadata ); |
||
| 168 | View Code Duplication | if ( $newLength < $oldLength - 5 ) { |
|
| 169 | $error++; |
||
| 170 | $this->output( "Warning: File:{$row->img_name} used to have " . |
||
| 171 | "$oldLength bytes of metadata but now has $newLength bytes. (forced)\n" ); |
||
| 172 | } |
||
| 173 | if ( $verbose ) { |
||
| 174 | $this->output( "Forcibly refreshed File:{$row->img_name}.\n" ); |
||
| 175 | } |
||
| 176 | } else { |
||
| 177 | if ( $verbose ) { |
||
| 178 | $this->output( "Skipping File:{$row->img_name}.\n" ); |
||
| 179 | } |
||
| 180 | } |
||
| 181 | } |
||
| 182 | } |
||
| 183 | $conds2 = [ 'img_name > ' . $dbw->addQuotes( $row->img_name ) ]; |
||
| 184 | wfWaitForSlaves(); |
||
| 185 | } while ( $res->numRows() === $this->mBatchSize ); |
||
| 186 | |||
| 187 | $total = $upgraded + $leftAlone; |
||
| 188 | if ( $force ) { |
||
| 189 | $this->output( "\nFinished refreshing file metadata for $total files. " |
||
| 190 | . "$upgraded needed to be refreshed, $leftAlone did not need to " |
||
| 191 | . "be but were refreshed anyways, and $error refreshes were suspicious.\n" ); |
||
| 192 | } else { |
||
| 193 | $this->output( "\nFinished refreshing file metadata for $total files. " |
||
| 194 | . "$upgraded were refreshed, $leftAlone were already up to date, " |
||
| 195 | . "and $error refreshes were suspicious.\n" ); |
||
| 196 | } |
||
| 197 | } |
||
| 198 | |||
| 199 | /** |
||
| 200 | * @param Database $dbw |
||
| 201 | * @return array |
||
| 202 | */ |
||
| 203 | function getConditions( $dbw ) { |
||
| 230 | |||
| 231 | /** |
||
| 232 | * @param bool $force |
||
| 233 | * @param bool $brokenOnly |
||
| 234 | */ |
||
| 235 | function setupParameters( $force, $brokenOnly ) { |
||
| 248 | } |
||
| 249 | |||
| 252 |
Loading history...
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.