SiteNotificationFactory::create() - Code Metrics - Inspection of "Merge pull request #10785 from hypeJunction/notifi..." - Elgg/Elgg - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( 35e3d0...d1d37e )

by Jeroen

created 2017-03-14 14:26 UTC

SiteNotificationFactory::create() C

↳ Parent: SiteNotificationFactory

Complexity

Conditions	7
Paths	12

Size

Total Lines	33
Code Lines	22

Duplication

Lines	0
Ratio	0 %

Importance

Changes

Metric	Value
cc	7
eloc	22
nc	12
nop	5
dl	0
loc	33
rs	6.7272
c	0
b	0
f	0

<?php

abstract class SiteNotificationFactory {

	/**
	 * Create a site notification
	 *
	 * @param ElggUser   $recipient Recipient of the notification
	 * @param string     $message   Notification message
	 * @param ElggUser   $actor     User who caused the notification event
	 * @param ElggData   $object    Optional object involved in the notification event
	 * @param string     $url       Target URL
	 * @return SiteNotification|null
	 */
	public static function create($recipient, $message, $actor, $object = null, $url = null) {
		$note = new SiteNotification();
		$note->owner_guid = $recipient->guid;
		$note->container_guid = $recipient->guid;
		$note->access_id = ACCESS_PRIVATE;
		$note->description = $message;

		if (!isset($url) && $object) {
			// TODO Add support for setting an URL for a notification about a new relationship
			switch ($object->getType()) {
				case 'annotation':
					// Annotations do not have an URL so we use the entity URL
					$url = $object->getEntity()->getURL();
abstract class User
{
    /** @return string */
    abstract public function getPassword();
}

class MyUser extends 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.
    }
}
					break;
				default:
					$url = $object->getURL();
					break;
			}
		}

		if ($url && $url != elgg_get_site_url()) {
			$note->setURL($url);
		}
		
		$note->setRead(false);

		if ($note->save()) {
			$note->setActor($actor);
			return $note;
		} else {
			return null;
		}
	}
}


1			<?php
2
3			abstract class SiteNotificationFactory {
4
5			/**
6			* Create a site notification
7			*
8			* @param ElggUser $recipient Recipient of the notification
9			* @param string $message Notification message
10			* @param ElggUser $actor User who caused the notification event
11			* @param ElggData $object Optional object involved in the notification event
12			* @param string $url Target URL
13			* @return SiteNotification\|null
14			*/
15			public static function create($recipient, $message, $actor, $object = null, $url = null) {
16			$note = new SiteNotification();
17			$note->owner_guid = $recipient->guid;
18			$note->container_guid = $recipient->guid;
19			$note->access_id = ACCESS_PRIVATE;
20			$note->description = $message;
21
22			if (!isset($url) && $object) {
23			// TODO Add support for setting an URL for a notification about a new relationship
24			switch ($object->getType()) {
25			case 'annotation':
26			// Annotations do not have an URL so we use the entity URL
27			$url = $object->getEntity()->getURL();
			0 ignored issues – show Bug introduced 2017-02-22 11:56 UTC by Report Bug Copy Issue Report It seems like you code against a specific sub-type and not the parent class `ElggData` as the method `getEntity()` does only exist in the following sub-classes of `ElggData`: `ElggAnnotation`, `ElggExtender`, `ElggMetadata`. Maybe you want to `instanceof` check for one of these explicitly? Let’s take a look at an example: abstract class User { /** @return string / abstract public function getPassword(); } class MyUser extends 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 sub-classes 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 parent class: abstract class User { /** @return string / abstract public function getPassword(); /* @return string */ abstract public function getDisplayName(); } Loading history...
28			break;
29			default:
30			$url = $object->getURL();
31			break;
32			}
33			}
34
35			if ($url && $url != elgg_get_site_url()) {
36			$note->setURL($url);
37			}
38
39			$note->setRead(false);
40
41			if ($note->save()) {
42			$note->setActor($actor);
43			return $note;
44			} else {
45			return null;
46			}
47			}
48			}
49

Elgg / Elgg

Push — master ( 35e3d0...d1d37e )

SiteNotificationFactory::create() C

Complexity

Size

Duplication

Importance

Available Fixes

Duplication Side-by-Side

Filter issues like