NotificationBuilder::buildNotifications() - Code Metrics - Inspection of "Fixed sending messages" - zbox/UnifiedPush - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( e1b645...9ebc86 )

by Alexander

created 2016-02-23 06:51 UTC

NotificationBuilder::buildNotifications() B

↳ Parent: NotificationBuilder

Complexity

Conditions	5
Paths	12

Size

Total Lines	25
Code Lines	14

Duplication

Lines	0
Ratio	0 %

Importance

Changes	3
Bugs	1	Features	0

Metric	Value
c	3
b	1
f	0
dl	0
loc	25
rs	8.439
cc	5
eloc	14
nc	12
nop	0

<?php

/*
 * (c) Alexander Zhukov <[email protected]>
 *
 * For the full copyright and license information, please view the LICENSE
 * file that was distributed with this source code.
 */

namespace Zbox\UnifiedPush\Message;

use Zbox\UnifiedPush\Exception\MalformedNotificationException;
use Zbox\UnifiedPush\Utils\JsonEncoder;

/**
 * Class NotificationBuilder
 * @package Zbox\UnifiedPush\Message
 */
class NotificationBuilder
{
    /**
     * @var MessageInterface
     */
    private $message;

    /**
     * @var \ArrayIterator
     */
    private $notifications;

    /**
     * @param MessageInterface $message
     */
    public function __construct(MessageInterface $message)
    {
        $this->notifications = new \ArrayIterator();
        $this->message = $message;
        $this->buildNotifications();
    }

    /**
     * @return string
     */
    public function getNotification()
    {
        $collection = $this->notifications;

        if ($collection->valid()) {
            $notification = $collection->current();
            $collection->next();
            return $notification;
        }
        return null;
    }

    /**
     * Generates number of notifications by message recipient count
     * and notification service limitations
     *
     * @return $this
     */
    public function buildNotifications()
    {
        $message        = $this->message;
        $recipientQueue = new \SplQueue();

        while ($recipient = $message->getRecipient()) {
            $recipientChunk[] = $recipient;
foreach ($collection as $item) {
    $myArray['foo'] = $item->getFoo();

    if ($item->hasBar()) {
        $myArray['bar'] = $item->getBar();
    }

    // do something with $myArray
}

            if (count($recipientChunk) >= $message->getMaxRecipientsPerMessage()) {
                $recipientQueue->enqueue($recipientChunk);
function myFunction($a) {
    switch ($a) {
        case 'foo':
            $x = 1;
            break;

        case 'bar':
            $x = 2;
            break;
    }

    // $x is potentially undefined here.
    echo $x;
}
                $recipientChunk = [];
            }
        }

        if (!empty($recipientChunk)) {
            $recipientQueue->enqueue($recipientChunk);
        }

        while (!$recipientQueue->isEmpty()) {
            $notification = $this->buildNotification($recipientQueue->dequeue());
            $this->notifications->append($notification);
        }

        return $this;
    }

    /**
     * Returns validated and encoded message
     *
     * @param array $recipients
     * @return array
     */
    private function buildNotification($recipients)
    {
        $message         = $this->message;
        $messageData     = $message->createMessage($recipients);
interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

        if (is_array($messageData)) {
            $messageData = JsonEncoder::jsonEncode($messageData);
function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
        }

        $this->validatePayload($messageData);

        $notification = $message->packMessage($messageData, $recipients);
interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

        return $notification;
    }

    /**
     * Check if maximum size allowed for a notification payload exceeded
     *
     * @param string $payload
     * @throws MalformedNotificationException
     * @return $this
     */
    public function validatePayload($payload)
    {
        $message     = $this->message;
        $maxLength   = $message->getPayloadMaxLength();
        $messageType = $message->getMessageType();
interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

        if (strlen($payload) > $maxLength) {
            throw new MalformedNotificationException(
                sprintf(
                    "The maximum size allowed for '%s' notification payload is %d bytes",
                    $messageType,
                    $maxLength
                )
            );
        }
        return $this;
    }
}


1			<?php
2
3			/*
4			* (c) Alexander Zhukov <[email protected]>
5			*
6			* For the full copyright and license information, please view the LICENSE
7			* file that was distributed with this source code.
8			*/
9
10			namespace Zbox\UnifiedPush\Message;
11
12			use Zbox\UnifiedPush\Exception\MalformedNotificationException;
13			use Zbox\UnifiedPush\Utils\JsonEncoder;
14
15			/**
16			* Class NotificationBuilder
17			* @package Zbox\UnifiedPush\Message
18			*/
19			class NotificationBuilder
20			{
21			/**
22			* @var MessageInterface
23			*/
24			private $message;
25
26			/**
27			* @var \ArrayIterator
28			*/
29			private $notifications;
30
31			/**
32			* @param MessageInterface $message
33			*/
34			public function __construct(MessageInterface $message)
35			{
36			$this->notifications = new \ArrayIterator();
37			$this->message = $message;
38			$this->buildNotifications();
39			}
40
41			/**
42			* @return string
43			*/
44			public function getNotification()
45			{
46			$collection = $this->notifications;
47
48			if ($collection->valid()) {
49			$notification = $collection->current();
50			$collection->next();
51			return $notification;
52			}
53			return null;
54			}
55
56			/**
57			* Generates number of notifications by message recipient count
58			* and notification service limitations
59			*
60			* @return $this
61			*/
62			public function buildNotifications()
63			{
64			$message = $this->message;
65			$recipientQueue = new \SplQueue();
66
67			while ($recipient = $message->getRecipient()) {
68			$recipientChunk[] = $recipient;
			0 ignored issues – show Coding Style Comprehensibility introduced 2016-02-06 21:57 UTC by Report Bug Copy Issue Report `$recipientChunk` was never initialized. Although not strictly required by PHP, it is generally a good practice to add `$recipientChunk = array();` before regardless. Adding an explicit array definition is generally preferable to implicit array definition as it guarantees a stable state of the code. Let’s take a look at an example: foreach ($collection as $item) { $myArray['foo'] = $item->getFoo(); if ($item->hasBar()) { $myArray['bar'] = $item->getBar(); } // do something with $myArray } As you can see in this example, the array `$myArray` is initialized the first time when the foreach loop is entered. You can also see that the value of the `bar` key is only written conditionally; thus, its value might result from a previous iteration. This might or might not be intended. To make your intention clear, your code more readible and to avoid accidental bugs, we recommend to add an explicit initialization `$myArray = array()` either outside or inside the foreach loop. Loading history...
69
70			if (count($recipientChunk) >= $message->getMaxRecipientsPerMessage()) {
71			$recipientQueue->enqueue($recipientChunk);
			0 ignored issues – show Bug introduced 2016-02-23 06:53 UTC by Report Bug Copy Issue Report The variable `$recipientChunk` does not seem to be defined for all execution paths leading up to this point. If you define a variable conditionally, it can happen that it is not defined for all execution paths. Let’s take a look at an example: function myFunction($a) { switch ($a) { case 'foo': $x = 1; break; case 'bar': $x = 2; break; } // $x is potentially undefined here. echo $x; } In the above example, the variable `$x` is defined if you pass “foo” or “bar” as argument for `$a`. However, since the `switch` statement has no default case statement, if you pass any other value, the variable `$x` would be undefined. Available Fixes Check for existence of the variable explicitly: function myFunction($a) { switch ($a) { case 'foo': $x = 1; break; case 'bar': $x = 2; break; } if (isset($x)) { // Make sure it's always set. echo $x; } } Define a default value for the variable: function myFunction($a) { $x = ''; // Set a default which gets overridden for certain paths. switch ($a) { case 'foo': $x = 1; break; case 'bar': $x = 2; break; } echo $x; } Add a value for the missing path: function myFunction($a) { switch ($a) { case 'foo': $x = 1; break; case 'bar': $x = 2; break; // We add support for the missing case. default: $x = ''; break; } echo $x; } Loading history...
72			$recipientChunk = [];
73			}
74			}
75
76			if (!empty($recipientChunk)) {
77			$recipientQueue->enqueue($recipientChunk);
78			}
79
80			while (!$recipientQueue->isEmpty()) {
81			$notification = $this->buildNotification($recipientQueue->dequeue());
82			$this->notifications->append($notification);
83			}
84
85			return $this;
86			}
87
88			/**
89			* Returns validated and encoded message
90			*
91			* @param array $recipients
92			* @return array
93			*/
94			private function buildNotification($recipients)
95			{
96			$message = $this->message;
97			$messageData = $message->createMessage($recipients);
			0 ignored issues – show Bug introduced 2016-02-06 21:57 UTC by Report Bug Copy Issue Report It seems like you code against a concrete implementation and not the interface `Zbox\UnifiedPush\Message\MessageInterface` as the method `createMessage()` does only exist in the following implementations of said interface: `Zbox\UnifiedPush\Message\Type\APNS`, `Zbox\UnifiedPush\Message\Type\GCM`, `Zbox\UnifiedPush\Message\Type\MPNSBase`, `Zbox\UnifiedPush\Message\Type\MPNSRaw`, `Zbox\UnifiedPush\Message\Type\MPNSTile`, `Zbox\UnifiedPush\Message\Type\MPNSToast`. Let’s take a look at an example: interface User { /** @return string / public function getPassword(); } class MyUser implements User { public function getPassword() { // return something } public function getDisplayName() { // return some name. } } class AuthSystem { public function authenticate(User $user) { $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName())); // do something. } } In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break. Available Fixes Change the type-hint for the parameter: class AuthSystem { public function authenticate(MyUser $user) { / ... / } } Add an additional type-check: class AuthSystem { public function authenticate(User $user) { if ($user instanceof MyUser) { $this->logger->info(/* ... /); } // or alternatively if ( ! $user instanceof MyUser) { throw new \LogicException( '$user must be an instance of MyUser, ' .'other instances are not supported.' ); } } } Note:* PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case. Add the method to the interface: interface User { /** @return string / public function getPassword(); /* @return string */ public function getDisplayName(); } Loading history...
98
99			if (is_array($messageData)) {
100			$messageData = JsonEncoder::jsonEncode($messageData);
			0 ignored issues – show Documentation introduced 2016-02-23 06:53 UTC by Report Bug Copy Issue Report `$messageData` is of type `array`, but the function expects a `string`. It seems like the type of the argument is not accepted by the function/method which you are calling. In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug. We suggest to add an explicit type cast like in the following example: function acceptsInteger($int) { } $x = '123'; // string "123" // Instead of acceptsInteger($x); // we recommend to use acceptsInteger((integer) $x); Loading history...
101			}
102
103			$this->validatePayload($messageData);
104
105			$notification = $message->packMessage($messageData, $recipients);
			0 ignored issues – show Bug introduced 2016-02-06 21:57 UTC by Report Bug Copy Issue Report It seems like you code against a concrete implementation and not the interface `Zbox\UnifiedPush\Message\MessageInterface` as the method `packMessage()` does only exist in the following implementations of said interface: `Zbox\UnifiedPush\Message\Type\APNS`, `Zbox\UnifiedPush\Message\Type\GCM`, `Zbox\UnifiedPush\Message\Type\MPNSBase`, `Zbox\UnifiedPush\Message\Type\MPNSRaw`, `Zbox\UnifiedPush\Message\Type\MPNSTile`, `Zbox\UnifiedPush\Message\Type\MPNSToast`. Let’s take a look at an example: interface User { /** @return string / public function getPassword(); } class MyUser implements User { public function getPassword() { // return something } public function getDisplayName() { // return some name. } } class AuthSystem { public function authenticate(User $user) { $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName())); // do something. } } In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break. Available Fixes Change the type-hint for the parameter: class AuthSystem { public function authenticate(MyUser $user) { / ... / } } Add an additional type-check: class AuthSystem { public function authenticate(User $user) { if ($user instanceof MyUser) { $this->logger->info(/* ... /); } // or alternatively if ( ! $user instanceof MyUser) { throw new \LogicException( '$user must be an instance of MyUser, ' .'other instances are not supported.' ); } } } Note:* PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case. Add the method to the interface: interface User { /** @return string / public function getPassword(); /* @return string */ public function getDisplayName(); } Loading history...
106
107			return $notification;
108			}
109
110			/**
111			* Check if maximum size allowed for a notification payload exceeded
112			*
113			* @param string $payload
114			* @throws MalformedNotificationException
115			* @return $this
116			*/
117			public function validatePayload($payload)
118			{
119			$message = $this->message;
120			$maxLength = $message->getPayloadMaxLength();
121			$messageType = $message->getMessageType();
			0 ignored issues – show Bug introduced 2016-02-06 21:57 UTC by Report Bug Copy Issue Report It seems like you code against a concrete implementation and not the interface `Zbox\UnifiedPush\Message\MessageInterface` as the method `getMessageType()` does only exist in the following implementations of said interface: `Zbox\UnifiedPush\Message\Type\APNS`, `Zbox\UnifiedPush\Message\Type\GCM`, `Zbox\UnifiedPush\Message\Type\MPNSBase`, `Zbox\UnifiedPush\Message\Type\MPNSRaw`, `Zbox\UnifiedPush\Message\Type\MPNSTile`, `Zbox\UnifiedPush\Message\Type\MPNSToast`. Let’s take a look at an example: interface User { /** @return string / public function getPassword(); } class MyUser implements User { public function getPassword() { // return something } public function getDisplayName() { // return some name. } } class AuthSystem { public function authenticate(User $user) { $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName())); // do something. } } In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break. Available Fixes Change the type-hint for the parameter: class AuthSystem { public function authenticate(MyUser $user) { / ... / } } Add an additional type-check: class AuthSystem { public function authenticate(User $user) { if ($user instanceof MyUser) { $this->logger->info(/* ... /); } // or alternatively if ( ! $user instanceof MyUser) { throw new \LogicException( '$user must be an instance of MyUser, ' .'other instances are not supported.' ); } } } Note:* PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case. Add the method to the interface: interface User { /** @return string / public function getPassword(); /* @return string */ public function getDisplayName(); } Loading history...
122
123			if (strlen($payload) > $maxLength) {
124			throw new MalformedNotificationException(
125			sprintf(
126			"The maximum size allowed for '%s' notification payload is %d bytes",
127			$messageType,
128			$maxLength
129			)
130			);
131			}
132			return $this;
133			}
134			}
135

zbox / UnifiedPush

GitHub Access Token became invalid

Push — master ( e1b645...9ebc86 )

NotificationBuilder::buildNotifications() B

Complexity

Size

Duplication

Importance

Available Fixes

Available Fixes

Available Fixes

Available Fixes

Duplication Side-by-Side

Filter issues like