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 PaymentRepository 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 PaymentRepository, and based on these observations, apply Extract Interface, too.
| 1 | <?php namespace BB\Repo; |
||
| 9 | class PaymentRepository extends DBRepository |
||
| 10 | { |
||
| 11 | |||
| 12 | /** |
||
| 13 | * @var Payment |
||
| 14 | */ |
||
| 15 | protected $model; |
||
| 16 | |||
| 17 | public static $SUBSCRIPTION = 'subscription'; |
||
| 18 | public static $INDUCTION = 'induction'; |
||
| 19 | |||
| 20 | private $reason = null; |
||
| 21 | private $source = null; |
||
| 22 | |||
| 23 | /** |
||
| 24 | * @param Payment $model |
||
| 25 | */ |
||
| 26 | public function __construct(Payment $model) |
||
| 31 | |||
| 32 | |||
| 33 | public function getPaginated(array $params) |
||
| 58 | |||
| 59 | |||
| 60 | public function getTotalAmount() |
||
| 82 | |||
| 83 | |||
| 84 | /** |
||
| 85 | * Record a payment against a user record |
||
| 86 | * |
||
| 87 | * @param string $reason What was the reason. subscription, induction, etc... |
||
| 88 | * @param int $userId The users ID |
||
| 89 | * @param string $source gocardless, paypal |
||
| 90 | * @param string $sourceId A reference for the source |
||
| 91 | * @param double $amount Amount received before a fee |
||
| 92 | * @param string $status paid, pending, cancelled, refunded |
||
| 93 | * @param double $fee The fee charged by the payment provider |
||
| 94 | * @param string $ref |
||
| 95 | * @param Carbon $paidDate |
||
| 96 | * @return int The ID of the payment record |
||
| 97 | */ |
||
| 98 | public function recordPayment($reason, $userId, $source, $sourceId, $amount, $status = 'paid', $fee = 0.0, $ref = '', Carbon $paidDate = null) |
||
| 129 | |||
| 130 | /** |
||
| 131 | * Record a subscription payment |
||
| 132 | * |
||
| 133 | * @param int $userId The users ID |
||
| 134 | * @param string $source gocardless, paypal |
||
| 135 | * @param string $sourceId A reference for the source |
||
| 136 | * @param double $amount Amount received before a fee |
||
| 137 | * @param string $status paid, pending, cancelled, refunded |
||
| 138 | * @param double $fee The fee charged by the payment provider |
||
| 139 | * @param string|null $ref |
||
| 140 | * @param Carbon $paidDate |
||
| 141 | * @return int The ID of the payment record |
||
| 142 | */ |
||
| 143 | public function recordSubscriptionPayment($userId, $source, $sourceId, $amount, $status = 'paid', $fee = 0.0, $ref = '', Carbon $paidDate = null) |
||
| 147 | |||
| 148 | /** |
||
| 149 | * An existing payment has been set to paid |
||
| 150 | * |
||
| 151 | * @param $paymentId |
||
| 152 | * @param Carbon $paidDate |
||
| 153 | */ |
||
| 154 | public function markPaymentPaid($paymentId, $paidDate) |
||
| 163 | |||
| 164 | /** |
||
| 165 | * Record a payment failure or cancellation |
||
| 166 | * |
||
| 167 | * @param int $paymentId |
||
| 168 | * @param string $status |
||
| 169 | */ |
||
| 170 | public function recordPaymentFailure($paymentId, $status = 'failed') |
||
| 178 | |||
| 179 | /** |
||
| 180 | * Assign an unassigned payment to a user |
||
| 181 | * |
||
| 182 | * @param int $paymentId |
||
| 183 | * @param int $userId |
||
| 184 | * |
||
| 185 | * @throws PaymentException |
||
| 186 | */ |
||
| 187 | public function assignPaymentToUser($paymentId, $userId) |
||
| 197 | |||
| 198 | public function refundPaymentToBalance($paymentId) |
||
| 210 | |||
| 211 | |||
| 212 | /** |
||
| 213 | * Fetch the users latest payment of a particular type |
||
| 214 | * @param integer $userId |
||
| 215 | * @param string $reason |
||
| 216 | * @return mixed |
||
| 217 | */ |
||
| 218 | View Code Duplication | public function latestUserPayment($userId, $reason = 'subscription') |
|
| 225 | |||
| 226 | |||
| 227 | /** |
||
| 228 | * Get all user payments of a specific reason |
||
| 229 | * @param $userId |
||
| 230 | * @param string $reason |
||
| 231 | * @return mixed |
||
| 232 | */ |
||
| 233 | View Code Duplication | public function getUserPaymentsByReason($userId, $reason) |
|
| 240 | |||
| 241 | |||
| 242 | /** |
||
| 243 | * @param string $source |
||
| 244 | */ |
||
| 245 | View Code Duplication | public function getUserPaymentsBySource($userId, $source) |
|
| 252 | |||
| 253 | /** |
||
| 254 | * Get all payments with a specific reference |
||
| 255 | * @param string $reference |
||
| 256 | * @return \Illuminate\Database\Eloquent\Collection |
||
| 257 | */ |
||
| 258 | public function getPaymentsByReference($reference) |
||
| 262 | |||
| 263 | /** |
||
| 264 | * @param string $referencePrefix |
||
| 265 | * @return \Illuminate\Database\Eloquent\Collection |
||
| 266 | */ |
||
| 267 | public function getEquipmentFeePayments($referencePrefix) |
||
| 273 | |||
| 274 | |||
| 275 | /** |
||
| 276 | * Return a paginated list of balance affecting payment for a user |
||
| 277 | * @param $userId |
||
| 278 | * @return \Illuminate\Database\Eloquent\Collection |
||
| 279 | */ |
||
| 280 | View Code Duplication | public function getBalancePaymentsPaginated($userId) |
|
| 287 | |||
| 288 | |||
| 289 | /** |
||
| 290 | * Return a collection of payments specifically for storage boxes |
||
| 291 | * @param integer $userId |
||
| 292 | * @return mixed |
||
| 293 | */ |
||
| 294 | View Code Duplication | public function getStorageBoxPayments($userId) |
|
| 301 | |||
| 302 | public function dateFilter($startDate, $endDate) |
||
| 307 | |||
| 308 | private function hasDateFilter() |
||
| 312 | |||
| 313 | /** |
||
| 314 | * Delete a record |
||
| 315 | * @param $recordId |
||
| 316 | * @return bool|null |
||
| 317 | * @throws \Exception |
||
| 318 | */ |
||
| 319 | public function delete($recordId) |
||
| 330 | |||
| 331 | public function canDelete($recordId) |
||
| 335 | |||
| 336 | /** |
||
| 337 | * Used for the getPaginated and getTotalAmount method |
||
| 338 | * @param $reasonFilter |
||
| 339 | */ |
||
| 340 | public function reasonFilter($reasonFilter) |
||
| 344 | |||
| 345 | private function hasReasonFilter() |
||
| 349 | |||
| 350 | /** |
||
| 351 | * Used for the getPaginated and getTotalAmount method |
||
| 352 | * @param string $sourceFilter |
||
| 353 | */ |
||
| 354 | public function sourceFilter($sourceFilter) |
||
| 358 | |||
| 359 | |||
| 360 | private function hasSourceFilter() |
||
| 364 | |||
| 365 | /** |
||
| 366 | * Used for the getPaginated and getTotalAmount method |
||
| 367 | */ |
||
| 368 | public function resetFilters() |
||
| 376 | |||
| 377 | /** |
||
| 378 | * Fetch a payment record using the id provided by the payment provider |
||
| 379 | * |
||
| 380 | * @param $sourceId |
||
| 381 | * @return Payment |
||
| 382 | */ |
||
| 383 | public function getPaymentBySourceId($sourceId) |
||
| 387 | |||
| 388 | /** |
||
| 389 | * Record a balance payment transfer between two users |
||
| 390 | * |
||
| 391 | * @param integer $sourceUserId |
||
| 392 | * @param integer $targetUserId |
||
| 393 | * @param double $amount |
||
| 394 | */ |
||
| 395 | public function recordBalanceTransfer($sourceUserId, $targetUserId, $amount) |
||
| 405 | } |
Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.
You can also find more detailed suggestions in the “Code” section of your repository.