nanasess /
ec-cube
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 CustomerRepository 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 CustomerRepository, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 32 | class CustomerRepository extends AbstractRepository |
||
| 33 | { |
||
| 34 | /** |
||
| 35 | * @var Queries |
||
| 36 | */ |
||
| 37 | protected $queries; |
||
| 38 | |||
| 39 | /** |
||
| 40 | * @var EntityManagerInterface |
||
| 41 | */ |
||
| 42 | protected $entityManager; |
||
| 43 | |||
| 44 | /** |
||
| 45 | * @var OrderRepository |
||
| 46 | */ |
||
| 47 | protected $orderRepository; |
||
| 48 | |||
| 49 | /** |
||
| 50 | * @var EccubeConfig |
||
| 51 | */ |
||
| 52 | protected $eccubeConfig; |
||
| 53 | |||
| 54 | /** |
||
| 55 | * @var EncoderFactoryInterface |
||
| 56 | */ |
||
| 57 | protected $encoderFactory; |
||
| 58 | |||
| 59 | /** |
||
| 60 | * CustomerRepository constructor. |
||
| 61 | * |
||
| 62 | * @param RegistryInterface $registry |
||
| 63 | * @param Queries $queries |
||
| 64 | * @param EntityManagerInterface $entityManager |
||
| 65 | * @param OrderRepository $orderRepository |
||
| 66 | * @param EncoderFactoryInterface $encoderFactory |
||
| 67 | * @param EccubeConfig $eccubeConfig |
||
| 68 | */ |
||
| 69 | 756 | public function __construct( |
|
| 85 | |||
| 86 | 8 | public function newCustomer() |
|
| 97 | |||
| 98 | 40 | public function getQueryBuilderBySearchData($searchData) |
|
| 249 | |||
| 250 | /** |
||
| 251 | * ユニークなシークレットキーを返す. |
||
| 252 | * |
||
| 253 | * @return string |
||
| 254 | */ |
||
| 255 | 303 | View Code Duplication | public function getUniqueSecretKey() |
| 264 | |||
| 265 | /** |
||
| 266 | * ユニークなパスワードリセットキーを返す |
||
| 267 | * |
||
| 268 | * @return string |
||
| 269 | */ |
||
| 270 | View Code Duplication | public function getUniqueResetKey() |
|
| 279 | |||
| 280 | /** |
||
| 281 | * 仮会員をシークレットキーで検索する. |
||
| 282 | * |
||
| 283 | * @param $secretKey |
||
| 284 | * |
||
| 285 | * @return null|Customer 見つからない場合はnullを返す. |
||
| 286 | */ |
||
| 287 | 4 | public function getProvisionalCustomerBySecretKey($secretKey) |
|
| 294 | |||
| 295 | /** |
||
| 296 | * 本会員をemailで検索する. |
||
| 297 | * |
||
| 298 | * @param $email |
||
| 299 | * |
||
| 300 | * @return null|Customer 見つからない場合はnullを返す. |
||
| 301 | */ |
||
| 302 | 1 | public function getRegularCustomerByEmail($email) |
|
| 309 | |||
| 310 | /** |
||
| 311 | * 本会員をリセットキーで検索する. |
||
| 312 | * |
||
| 313 | * @param $resetKey |
||
| 314 | * |
||
| 315 | * @return null|Customer 見つからない場合はnullを返す. |
||
| 316 | */ |
||
| 317 | 3 | public function getRegularCustomerByResetKey($resetKey) |
|
| 328 | |||
| 329 | /** |
||
| 330 | * リセット用パスワードを生成する. |
||
| 331 | * |
||
| 332 | * @return string |
||
| 333 | */ |
||
| 334 | 2 | public function getResetPassword() |
|
| 338 | |||
| 339 | /** |
||
| 340 | * 会員の初回購入時間、購入時間、購入回数、購入金額を更新する |
||
| 341 | * |
||
| 342 | * @param Customer $Customer |
||
| 343 | * @param null $isNewOrder |
||
| 344 | */ |
||
| 345 | 1 | public function updateBuyData(Customer $Customer, $isNewOrder = null) |
|
| 346 | { |
||
| 347 | // 会員の場合、初回購入時間・購入時間・購入回数・購入金額を更新 |
||
| 348 | |||
| 349 | $arr = [ |
||
| 350 | 1 | OrderStatus::NEW, |
|
| 351 | OrderStatus::PAY_WAIT, |
||
| 352 | OrderStatus::BACK_ORDER, |
||
| 353 | OrderStatus::DELIVERED, |
||
| 354 | OrderStatus::PAID, |
||
| 355 | ]; |
||
| 356 | |||
| 357 | 1 | $result = $this->orderRepository->getCustomerCount($Customer, $arr); |
|
| 358 | |||
| 359 | 1 | if (!empty($result)) { |
|
| 360 | 1 | $data = $result[0]; |
|
| 361 | |||
| 362 | 1 | $now = new \DateTime(); |
|
| 363 | |||
| 364 | 1 | $firstBuyDate = $Customer->getFirstBuyDate(); |
|
| 365 | 1 | if (empty($firstBuyDate)) { |
|
| 366 | 1 | $Customer->setFirstBuyDate($now); |
|
| 367 | } |
||
| 368 | |||
| 369 | 1 | if ($isNewOrder) { |
|
| 370 | 1 | $Customer->setLastBuyDate($now); |
|
| 371 | } else { |
||
| 372 | $Order = $this->orderRepository->find($data['order_id']); |
||
| 373 | if ($Order) { |
||
| 374 | $Customer->setLastBuyDate($Order->getOrderDate()); |
||
| 375 | } |
||
| 376 | } |
||
| 377 | |||
| 378 | 1 | $Customer->setBuyTimes($data['buy_times']); |
|
| 379 | 1 | $Customer->setBuyTotal($data['buy_total']); |
|
| 380 | } else { |
||
| 381 | // 受注データが存在しなければ初期化 |
||
| 382 | 1 | $Customer->setFirstBuyDate(null); |
|
| 383 | 1 | $Customer->setLastBuyDate(null); |
|
| 384 | 1 | $Customer->setBuyTimes(0); |
|
| 385 | 1 | $Customer->setBuyTotal(0); |
|
| 386 | } |
||
| 387 | |||
| 388 | 1 | $this->entityManager->persist($Customer); |
|
| 389 | 1 | $this->entityManager->flush(); |
|
| 390 | } |
||
| 391 | |||
| 392 | /** |
||
| 393 | * 仮会員, 本会員の会員を返す. |
||
| 394 | * Eccube\Entity\CustomerのUniqueEntityバリデーションで使用しています. |
||
| 395 | * |
||
| 396 | * @param array $criteria |
||
| 397 | * |
||
| 398 | * @return Customer[] |
||
| 399 | */ |
||
| 400 | 42 | public function getNonWithdrawingCustomers(array $criteria = []) |
|
| 409 | } |
||
| 410 |
Loading history...
If a method or function can return multiple different values and unless you are sure that you only can receive a single value in this context, we recommend to add an additional type check:
If this a common case that PHP Analyzer should handle natively, please let us know by opening an issue.