SubscriptionsManager::__construct() - Code Metrics - Inspection of "Check for first-time processing removed from Subsc..." - skobkin/point-tools - Measure and Improve Code Quality continuously with Scrutinizer

Test Failed

Push — master ( cc2916...fd3cd2 )

by Alexey

created 2017-01-08 16:00 UTC

SubscriptionsManager::__construct() A

↳ Parent: SubscriptionsManager

Complexity

Conditions	1
Paths	1

Size

Total Lines	5
Code Lines	3

Duplication

Lines	0
Ratio	0 %

Code Coverage

Tests	0
CRAP Score	2

Importance

Changes

Metric	Value
dl	0
loc	5
ccs	0
cts	5
cp	0
rs	9.4285
c	0
b	0
f	0
cc	1
eloc	3
nc	1
nop	2
crap	2

<?php

namespace Skobkin\Bundle\PointToolsBundle\Service;

use Doctrine\ORM\EntityManager;
use Skobkin\Bundle\PointToolsBundle\Entity\Subscription;
use Skobkin\Bundle\PointToolsBundle\Entity\SubscriptionEvent;
use Skobkin\Bundle\PointToolsBundle\Entity\User;
use Skobkin\Bundle\PointToolsBundle\Event\UserSubscribersUpdatedEvent;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;

class SubscriptionsManager
{
    /**
     * @var EntityManager
     */
    private $em;

    /**
     * @var EventDispatcherInterface
     */
    private $eventDispatcher;


    public function __construct(EntityManager $entityManager, EventDispatcherInterface $eventDispatcher)
function someFunction(ManagerRegistry $registry) {
    $em = $registry->getManager();
    $em->getConnection()->beginTransaction();
    try {
        // Do something.
        $em->getConnection()->commit();
    } catch (\Exception $ex) {
        $em->getConnection()->rollback();
        $em->close();

        throw $ex;
    }
}
    {
        $this->em = $entityManager;
        $this->eventDispatcher = $eventDispatcher;
    }

    /**
     * @param User $user
     * @param User[] $newSubscribersList
     */
    public function updateUserSubscribers(User $user, $newSubscribersList = [])
    {
        $tmpOldSubscribers = $user->getSubscribers();

        $oldSubscribersList = [];

        foreach ($tmpOldSubscribers as $subscription) {
$collection = json_decode($data, true);
if ( ! is_array($collection)) {
    throw new \RuntimeException('$collection must be an array.');
}

foreach ($collection as $item) { /** ... */ }
            $oldSubscribersList[] = $subscription->getSubscriber();
        }

        $subscribedList = $this->getUsersListsDiff($newSubscribersList, $oldSubscribersList);
        $unsubscribedList = $this->getUsersListsDiff($oldSubscribersList, $newSubscribersList);

        $this->processSubscribedUsers($user, $subscribedList);
        $this->processUnsubscribedUsers($user, $unsubscribedList);

        // Removing users from database
        // @todo Maybe remove via ORM
        $this->em->getRepository('SkobkinPointToolsBundle:Subscription')->removeSubscribers($user, $unsubscribedList);

        $this->dispatchSubscribersUpdatedEvent($user, $subscribedList, $unsubscribedList);
    }

    /**
     * Compares $list1 against $list2 and returns the values in $list1 that are not present in $list2.
     *
     * @param User[] $list1
     * @param User[] $list2
     *
     * @return User[]
     */
    public function getUsersListsDiff(array $list1 = [], array $list2 = []): array
    {
        $hash1 = [];
        $hash2 = [];

        foreach ($list1 as $user) {
            $hash1[$user->getId()] = $user;
        }
        foreach ($list2 as $user) {
            $hash2[$user->getId()] = $user;
        }

        return array_diff_key($hash1, $hash2);
    }

    /**
     * @param User $user
     * @param User[] $subscribers
     */
    private function processSubscribedUsers(User $user, array $subscribers)
    {
        foreach ($subscribers as $subscriber) {
            $subscription = new Subscription($user, $subscriber);

            $user->addSubscriber($subscription);
            $this->em->persist($subscription);

            $logEvent = new SubscriptionEvent($user, $subscriber, SubscriptionEvent::ACTION_SUBSCRIBE);
            $this->em->persist($logEvent);

            $user->addNewSubscriberEvent($logEvent);
        }
    }

    /**
     * @param User $user
     * @param User[] $subscribers
     */
    private function processUnsubscribedUsers(User $user, array $subscribers)
    {
        foreach ($subscribers as $subscriber) {
            $logEvent = new SubscriptionEvent($user, $subscriber, SubscriptionEvent::ACTION_UNSUBSCRIBE);
            $this->em->persist($logEvent);

            $user->addNewSubscriberEvent($logEvent);
        }
    }

    /**
     * @param User $user
     * @param User[] $subscribed
     * @param User[] $unsubscribed
     */
    private function dispatchSubscribersUpdatedEvent(User $user, array $subscribed, array $unsubscribed)
    {
        if (0 !== count($subscribed) || 0 !== count($unsubscribed)) {
            // Dispatching event
            $subscribersUpdatedEvent = new UserSubscribersUpdatedEvent($user, $subscribed, $unsubscribed);
            $this->eventDispatcher->dispatch(UserSubscribersUpdatedEvent::NAME, $subscribersUpdatedEvent);
        }
    }
}

1			<?php
2
3			namespace Skobkin\Bundle\PointToolsBundle\Service;
4
5			use Doctrine\ORM\EntityManager;
6			use Skobkin\Bundle\PointToolsBundle\Entity\Subscription;
7			use Skobkin\Bundle\PointToolsBundle\Entity\SubscriptionEvent;
8			use Skobkin\Bundle\PointToolsBundle\Entity\User;
9			use Skobkin\Bundle\PointToolsBundle\Event\UserSubscribersUpdatedEvent;
10			use Symfony\Component\EventDispatcher\EventDispatcherInterface;
11
12			class SubscriptionsManager
13			{
14			/**
15			* @var EntityManager
16			*/
17			private $em;
18
19			/**
20			* @var EventDispatcherInterface
21			*/
22			private $eventDispatcher;
23
24
25			public function __construct(EntityManager $entityManager, EventDispatcherInterface $eventDispatcher)
			0 ignored issues – show Bug introduced 2017-01-07 18:50 UTC by Report Bug Copy Issue Report You have injected the EntityManager via parameter `$entityManager`. This is generally not recommended as it might get closed and become unusable. Instead, it is recommended to inject the ManagerRegistry and retrieve the EntityManager via `getManager()` each time you need it. The `EntityManager` might become unusable for example if a transaction is rolled back and it gets closed. Let’s assume that somewhere in your application, or in a third-party library, there is code such as the following: function someFunction(ManagerRegistry $registry) { $em = $registry->getManager(); $em->getConnection()->beginTransaction(); try { // Do something. $em->getConnection()->commit(); } catch (\Exception $ex) { $em->getConnection()->rollback(); $em->close(); throw $ex; } } If that code throws an exception and the `EntityManager` is closed. Any other code which depends on the same instance of the `EntityManager` during this request will fail. On the other hand, if you instead inject the `ManagerRegistry`, the `getManager()` method guarantees that you will always get a usable manager instance. Loading history...
26			{
27			$this->em = $entityManager;
28			$this->eventDispatcher = $eventDispatcher;
29			}
30
31			/**
32			* @param User $user
33			* @param User[] $newSubscribersList
34			*/
35			public function updateUserSubscribers(User $user, $newSubscribersList = [])
36			{
37			$tmpOldSubscribers = $user->getSubscribers();
38
39			$oldSubscribersList = [];
40
41			foreach ($tmpOldSubscribers as $subscription) {
			0 ignored issues – show Bug introduced 2017-01-08 16:03 UTC by Report Bug Copy Issue Report The expression `$tmpOldSubscribers` of type `object<Skobkin\Bundle\Po...ctions\ArrayCollection>` is not guaranteed to be traversable. How about adding an additional type check? There are different options of fixing this problem. If you want to be on the safe side, you can add an additional type-check: $collection = json_decode($data, true); if ( ! is_array($collection)) { throw new \RuntimeException('$collection must be an array.'); } foreach ($collection as $item) { /** ... / } If you are sure that the expression is traversable, you might want to add a doc comment cast* to improve IDE auto-completion and static analysis: /** @var array $collection / $collection = json_decode($data, true); foreach ($collection as $item) { /* .. / } Mark the issue as a false-positive*: Just hover the remove button, in the top-right corner of this issue for more options. Loading history...
42			$oldSubscribersList[] = $subscription->getSubscriber();
43			}
44
45			$subscribedList = $this->getUsersListsDiff($newSubscribersList, $oldSubscribersList);
46			$unsubscribedList = $this->getUsersListsDiff($oldSubscribersList, $newSubscribersList);
47
48			$this->processSubscribedUsers($user, $subscribedList);
49			$this->processUnsubscribedUsers($user, $unsubscribedList);
50
51			// Removing users from database
52			// @todo Maybe remove via ORM
53			$this->em->getRepository('SkobkinPointToolsBundle:Subscription')->removeSubscribers($user, $unsubscribedList);
54
55			$this->dispatchSubscribersUpdatedEvent($user, $subscribedList, $unsubscribedList);
56			}
57
58			/**
59			* Compares $list1 against $list2 and returns the values in $list1 that are not present in $list2.
60			*
61			* @param User[] $list1
62			* @param User[] $list2
63			*
64			* @return User[]
65			*/
66			public function getUsersListsDiff(array $list1 = [], array $list2 = []): array
67			{
68			$hash1 = [];
69			$hash2 = [];
70
71			foreach ($list1 as $user) {
72			$hash1[$user->getId()] = $user;
73			}
74			foreach ($list2 as $user) {
75			$hash2[$user->getId()] = $user;
76			}
77
78			return array_diff_key($hash1, $hash2);
79			}
80
81			/**
82			* @param User $user
83			* @param User[] $subscribers
84			*/
85			private function processSubscribedUsers(User $user, array $subscribers)
86			{
87			foreach ($subscribers as $subscriber) {
88			$subscription = new Subscription($user, $subscriber);
89
90			$user->addSubscriber($subscription);
91			$this->em->persist($subscription);
92
93			$logEvent = new SubscriptionEvent($user, $subscriber, SubscriptionEvent::ACTION_SUBSCRIBE);
94			$this->em->persist($logEvent);
95
96			$user->addNewSubscriberEvent($logEvent);
97			}
98			}
99
100			/**
101			* @param User $user
102			* @param User[] $subscribers
103			*/
104			private function processUnsubscribedUsers(User $user, array $subscribers)
105			{
106			foreach ($subscribers as $subscriber) {
107			$logEvent = new SubscriptionEvent($user, $subscriber, SubscriptionEvent::ACTION_UNSUBSCRIBE);
108			$this->em->persist($logEvent);
109
110			$user->addNewSubscriberEvent($logEvent);
111			}
112			}
113
114			/**
115			* @param User $user
116			* @param User[] $subscribed
117			* @param User[] $unsubscribed
118			*/
119			private function dispatchSubscribersUpdatedEvent(User $user, array $subscribed, array $unsubscribed)
120			{
121			if (0 !== count($subscribed) \|\| 0 !== count($unsubscribed)) {
122			// Dispatching event
123			$subscribersUpdatedEvent = new UserSubscribersUpdatedEvent($user, $subscribed, $unsubscribed);
124			$this->eventDispatcher->dispatch(UserSubscribersUpdatedEvent::NAME, $subscribersUpdatedEvent);
125			}
126			}
127			}

skobkin / point-tools

Push — master ( cc2916...fd3cd2 )

SubscriptionsManager::__construct() A

Complexity

Size

Duplication

Code Coverage

Importance

Duplication Side-by-Side

Filter issues like