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 CopyFileBackend 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 CopyFileBackend, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
37 | class CopyFileBackend extends Maintenance { |
||
38 | /** @var array|null (path sha1 => stat) Pre-computed dst stat entries from listings */ |
||
39 | protected $statCache = null; |
||
40 | |||
41 | public function __construct() { |
||
56 | |||
57 | public function execute() { |
||
58 | $src = FileBackendGroup::singleton()->get( $this->getOption( 'src' ) ); |
||
59 | $dst = FileBackendGroup::singleton()->get( $this->getOption( 'dst' ) ); |
||
60 | $containers = explode( '|', $this->getOption( 'containers' ) ); |
||
61 | $subDir = rtrim( $this->getOption( 'subdir', '' ), '/' ); |
||
62 | |||
63 | $rateFile = $this->getOption( 'ratefile' ); |
||
64 | |||
65 | foreach ( $containers as $container ) { |
||
66 | if ( $subDir != '' ) { |
||
67 | $backendRel = "$container/$subDir"; |
||
68 | $this->output( "Doing container '$container', directory '$subDir'...\n" ); |
||
69 | } else { |
||
70 | $backendRel = $container; |
||
71 | $this->output( "Doing container '$container'...\n" ); |
||
72 | } |
||
73 | |||
74 | if ( $this->hasOption( 'missingonly' ) ) { |
||
75 | $this->output( "\tBuilding list of missing files..." ); |
||
76 | $srcPathsRel = $this->getListingDiffRel( $src, $dst, $backendRel ); |
||
77 | $this->output( count( $srcPathsRel ) . " file(s) need to be copied.\n" ); |
||
78 | } else { |
||
79 | $srcPathsRel = $src->getFileList( [ |
||
80 | 'dir' => $src->getRootStoragePath() . "/$backendRel", |
||
81 | 'adviseStat' => true // avoid HEADs |
||
82 | ] ); |
||
83 | if ( $srcPathsRel === null ) { |
||
84 | $this->error( "Could not list files in $container.", 1 ); // die |
||
85 | } |
||
86 | } |
||
87 | |||
88 | if ( $this->getOption( 'prestat' ) && !$this->hasOption( 'missingonly' ) ) { |
||
89 | // Build the stat cache for the destination files |
||
90 | $this->output( "\tBuilding destination stat cache..." ); |
||
91 | $dstPathsRel = $dst->getFileList( [ |
||
92 | 'dir' => $dst->getRootStoragePath() . "/$backendRel", |
||
93 | 'adviseStat' => true // avoid HEADs |
||
94 | ] ); |
||
95 | if ( $dstPathsRel === null ) { |
||
96 | $this->error( "Could not list files in $container.", 1 ); // die |
||
97 | } |
||
98 | $this->statCache = []; |
||
99 | foreach ( $dstPathsRel as $dstPathRel ) { |
||
100 | $path = $dst->getRootStoragePath() . "/$backendRel/$dstPathRel"; |
||
101 | $this->statCache[sha1( $path )] = $dst->getFileStat( [ 'src' => $path ] ); |
||
102 | } |
||
103 | $this->output( "done [" . count( $this->statCache ) . " file(s)]\n" ); |
||
104 | } |
||
105 | |||
106 | $this->output( "\tCopying file(s)...\n" ); |
||
107 | $count = 0; |
||
108 | $batchPaths = []; |
||
109 | View Code Duplication | foreach ( $srcPathsRel as $srcPathRel ) { |
|
110 | // Check up on the rate file periodically to adjust the concurrency |
||
111 | if ( $rateFile && ( !$count || ( $count % 500 ) == 0 ) ) { |
||
112 | $this->mBatchSize = max( 1, (int)file_get_contents( $rateFile ) ); |
||
113 | $this->output( "\tBatch size is now {$this->mBatchSize}.\n" ); |
||
114 | } |
||
115 | $batchPaths[$srcPathRel] = 1; // remove duplicates |
||
116 | if ( count( $batchPaths ) >= $this->mBatchSize ) { |
||
117 | $this->copyFileBatch( array_keys( $batchPaths ), $backendRel, $src, $dst ); |
||
118 | $batchPaths = []; // done |
||
119 | } |
||
120 | ++$count; |
||
121 | } |
||
122 | if ( count( $batchPaths ) ) { // left-overs |
||
123 | $this->copyFileBatch( array_keys( $batchPaths ), $backendRel, $src, $dst ); |
||
124 | $batchPaths = []; // done |
||
125 | } |
||
126 | $this->output( "\tCopied $count file(s).\n" ); |
||
127 | |||
128 | if ( $this->hasOption( 'syncviadelete' ) ) { |
||
129 | $this->output( "\tBuilding list of excess destination files..." ); |
||
130 | $delPathsRel = $this->getListingDiffRel( $dst, $src, $backendRel ); |
||
131 | $this->output( count( $delPathsRel ) . " file(s) need to be deleted.\n" ); |
||
132 | |||
133 | $this->output( "\tDeleting file(s)...\n" ); |
||
134 | $count = 0; |
||
135 | $batchPaths = []; |
||
136 | View Code Duplication | foreach ( $delPathsRel as $delPathRel ) { |
|
137 | // Check up on the rate file periodically to adjust the concurrency |
||
138 | if ( $rateFile && ( !$count || ( $count % 500 ) == 0 ) ) { |
||
139 | $this->mBatchSize = max( 1, (int)file_get_contents( $rateFile ) ); |
||
140 | $this->output( "\tBatch size is now {$this->mBatchSize}.\n" ); |
||
141 | } |
||
142 | $batchPaths[$delPathRel] = 1; // remove duplicates |
||
143 | if ( count( $batchPaths ) >= $this->mBatchSize ) { |
||
144 | $this->delFileBatch( array_keys( $batchPaths ), $backendRel, $dst ); |
||
145 | $batchPaths = []; // done |
||
146 | } |
||
147 | ++$count; |
||
148 | } |
||
149 | if ( count( $batchPaths ) ) { // left-overs |
||
150 | $this->delFileBatch( array_keys( $batchPaths ), $backendRel, $dst ); |
||
151 | $batchPaths = []; // done |
||
152 | } |
||
153 | |||
154 | $this->output( "\tDeleted $count file(s).\n" ); |
||
155 | } |
||
156 | |||
157 | if ( $subDir != '' ) { |
||
158 | $this->output( "Finished container '$container', directory '$subDir'.\n" ); |
||
159 | } else { |
||
160 | $this->output( "Finished container '$container'.\n" ); |
||
161 | } |
||
162 | } |
||
163 | |||
164 | $this->output( "Done.\n" ); |
||
165 | } |
||
166 | |||
167 | /** |
||
168 | * @param FileBackend $src |
||
169 | * @param FileBackend $dst |
||
170 | * @param string $backendRel |
||
171 | * @return array (rel paths in $src minus those in $dst) |
||
172 | */ |
||
173 | protected function getListingDiffRel( FileBackend $src, FileBackend $dst, $backendRel ) { |
||
201 | |||
202 | /** |
||
203 | * @param array $srcPathsRel |
||
204 | * @param string $backendRel |
||
205 | * @param FileBackend $src |
||
206 | * @param FileBackend $dst |
||
207 | * @return void |
||
208 | */ |
||
209 | protected function copyFileBatch( |
||
289 | |||
290 | /** |
||
291 | * @param array $dstPathsRel |
||
292 | * @param string $backendRel |
||
293 | * @param FileBackend $dst |
||
294 | * @return void |
||
295 | */ |
||
296 | protected function delFileBatch( |
||
326 | |||
327 | /** |
||
328 | * @param FileBackend $src |
||
329 | * @param FileBackend $dst |
||
330 | * @param string $sPath |
||
331 | * @param string $dPath |
||
332 | * @return bool |
||
333 | */ |
||
334 | protected function filesAreSame( FileBackend $src, FileBackend $dst, $sPath, $dPath ) { |
||
375 | } |
||
376 | |||
379 |
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.