SendMessages - Code Metrics - Inspection of "Bug: misc fixes after parameter type changed" - satrun77/tinyissue - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — develop ( 7a0090...cf7e5a )

by Mohamed

created 2016-06-18 11:40 UTC

SendMessages C

↳ Parent: Project

Complexity

Total Complexity

Size/Duplication

Total Lines	402
Duplicated Lines	2.24 %

Coupling/Cohesion

Components	2
Dependencies	8

Test Coverage

Coverage

93.92%

Importance

Changes	2
Bugs	1	Features	0

Metric	Value
wmc	56
c	2
b	1
f	0
lcom	2
cbo	8
dl	9
loc	402
ccs	139
cts	148
cp	0.9392
rs	6.5957

16 Methods

Rating	Name	Duplication	Size	Complexity
A	getIssue()	0	8	2
A	getProject()	0	4	1
A	getProjectId()	0	4	1
A	validateData()	0	9	3
A	populateData()	9	9	2
A	isStatusMessage()	0	5	4
A	createProjectUserObject()	0	11	1
A	getTags()	0	19	4
C	wantToReceiveMessage()	0	36	8
A	getMessageDataForAddIssue()	0	21	4
B	getMessageDataForUpdateIssue()	0	23	5
A	getMessageDataForReopenIssue()	0	14	3
A	getMessageDataForCloseIssue()	0	13	2
A	getMessageDataForAssignIssue()	0	13	2
A	getMessageDataForChangeTagIssue()	0	21	3
C	processDirectMessages()	0	65	11

How to fix Duplicated Code Complexity

Duplicated Code

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:

If you have the same expression in different places: Extract expression to a method
If you have the same method in different sub-classes: Extract method, and pull up field to the parent class
If you have the same code in unrelated classes: Consider extracting the code to a new class, and injecting that class

Complex Class

Tip: Before tackling complexity, make sure that you eliminate any duplication first. This often can reduce the size of classes significantly.

Complex classes like SendMessages 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 SendMessages, and based on these observations, apply Extract Interface, too.

<?php

/*
 * This file is part of the Tinyissue package.
 *
 * (c) Mohamed Alsharaf <[email protected]>
 *
 * For the full copyright and license information, please view the LICENSE
 * file that was distributed with this source code.
 */

namespace Tinyissue\Model\Project\Issue;

use Illuminate\Database\Eloquent\Collection;
use Tinyissue\Model\Message;
use Tinyissue\Model\Message\Queue;
use Tinyissue\Model\Project;
use Tinyissue\Model\Project\Issue;
use Tinyissue\Model\Tag;
use Tinyissue\Model\User;
use Tinyissue\Services\SendMessagesAbstract;

/**
 * SendMessages is a class to manage & process of sending messages about issue changes.
 *
 * @author Mohamed Alsharaf <[email protected]>
 */
class SendMessages extends SendMessagesAbstract
{
    protected $template = 'update_issue';

    /**
     * Collection of tags.
     *
     * @var Collection
     */
    protected $tags;

    /**
     * Returns an instance of Issue.
     *
     * @return bool
     */
    protected function getIssue()
    {
        if (null === $this->issue) {
            $this->issue = $this->getModel();
class Id
{
    public $id;

    public function __construct($id)
    {
        $this->id = $id;
    }

}

class Account
{
    /** @var  Id $id */
    public $id;
}

$account_id = false;

if (starsAreRight()) {
    $account_id = new Id(42);
}

$account = new Account();
if ($account instanceof Id)
{
    $account->id = $account_id;
}
        }

        return $this->issue;
class Author {
    private $name;

    public function __construct($name) {
        $this->name = $name;
    }

    public function getName() {
        return $this->name;
    }
}

abstract class Post {
    public function getAuthor() {
        return 'Johannes';
    }
}

class BlogPost extends Post {
    public function getAuthor() {
        return new Author('Johannes');
    }
}

class ForumPost extends Post { /* ... */ }

function my_function(Post $post) {
    echo strtoupper($post->getAuthor());
}
    }

    /**
     * Returns an instance of Project.
     *
     * @return Project
     */
    protected function getProject()
    {
        return $this->getIssue()->project;
    }

    /**
     * Returns the project id.
     *
     * @return int
     */
    protected function getProjectId()
    {
        return $this->getIssue()->project_id;
    }

    /**
     * Returns message data belongs to adding an issue.
     *
     * @param Queue $queue
     *
     * @return array
     */
    protected function getMessageDataForAddIssue(Queue $queue)
    {
        $messageData                    = ['changes' => []];
        $messageData['changeByHeading'] = $queue->changeBy->fullname . ' created a new issue';
<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}
        if ($this->issue->assigned) {

            $messageData['changes']['assignee'] = $this->issue->assigned->fullname;

        }

        $tags = $this->issue->tags()->with('parent')->get();
        foreach ($tags as $tag) {
            $tagArray                                   = $tag->toShortArray();
            $tagArray['now']                            = $tagArray['name'];
            $messageData['changes'][$tag->parent->name] = $tagArray;
        }

        if ($this->issue->time_quote) {
            $messageData['changes']['time_quote'] = \Html::duration($this->issue->time_quote);
        }

        return $messageData;
    }

    /**
     * Returns message data belongs to updating an issue.
     *
     * @param Queue $queue
     *
     * @return array
     */
    protected function getMessageDataForUpdateIssue(Queue $queue)
    {
        $messageData                    = [];
        $messageData['changeByHeading'] = $queue->changeBy->fullname . ' updated an issue';
<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}

        foreach ($queue->payload['dirty'] as $field => $value) {

            // Skip fields that not part of the white list
            if (!in_array($field,
                ['change_by', 'title', 'body', 'assignee', 'status', 'type', 'resolution', 'time_quote'])
            ) {
                continue;
            }
            // Format quote to readable time
            $value                          = $field === 'time_quote' ? \Html::duration($value) : $value;
            $value                          = $field === 'body' ? \Html::format($value) : $value;
            $messageData['changes'][$field] = [
                'now' => $value,
                'was' => $queue->getDataFromPayload('origin.' . $field),
            ];
        }

        return $messageData;
    }

    /**
     * Returns message data belongs to reopening an issue.
     *
     * @param Queue $queue
     *
     * @return array
     */
    protected function getMessageDataForReopenIssue(Queue $queue)
    {
        $messageData                    = [];
        $messageData['changeByHeading'] = $queue->changeBy->fullname . ' reopened an issue';
<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}
        $statusNow                      = $this->issue
            ->tags()->with('parent')->get()->where('parent.name', Tag::GROUP_STATUS)->last();
        $messageData['changes']['status'] = [
            'was' => trans('tinyissue.closed'),
            'now' => ($statusNow ? $statusNow->fullname : ''),
            'id'  => ($statusNow ? $statusNow->id : ''),
        ];

        return $messageData;
    }

    /**
     * Returns message data belongs to closing an issue.
     *
     * @param Queue $queue
     *
     * @return array
     */
    protected function getMessageDataForCloseIssue(Queue $queue)
    {
        $messageData                    = [];
        $messageData['changeByHeading'] = $queue->changeBy->fullname . ' closed an issue';
<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}
        $statusWas                      = $this->issue
            ->tags()->with('parent')->get()->where('parent.name', Tag::GROUP_STATUS)->last();
        $messageData['changes']['status'] = [
            'was' => ($statusWas ? $statusWas->fullname : ''),
            'now' => trans('tinyissue.closed'),
        ];

        return $messageData;
    }

    /**
     * Returns message data belongs to assigning an issue to a user.
     *
     * @param Queue $queue
     * @param array $extraData
     *
     * @return array
     */
    protected function getMessageDataForAssignIssue(Queue $queue, array $extraData)
    {
        $messageData = [];
        if (!array_key_exists('now', $extraData)) {
            $assignTo  = $this->getUserById($queue->getDataFromPayload('dirty.assigned_to'));
            $extraData = ['now' => $assignTo->fullname];
        }

        $messageData['changes']['assignee'] = $extraData;
        $messageData['changeByHeading']     = $queue->changeBy->fullname . ' assigned an issue to ' . $extraData['now'];
<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}

        return $messageData;
    }

    /**
     * Returns message data belongs to changing an issue tags.
     *
     * @param Queue $queue
     *
     * @return array
     */
    protected function getMessageDataForChangeTagIssue(Queue $queue)
    {
        $messageData                    = [];
        $messageData['changeByHeading'] = $queue->changeBy->fullname . ' changed an issue tag';
<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}

        foreach ($queue->payload['added'] as $tag) {

            $group                          = strtolower($tag['group']);
            $messageData['changes'][$group] = [
                'now'           => $tag['name'],
                'id'            => $tag['id'],
                'message_limit' => $tag['message_limit'],
            ];
        }

        foreach ($queue->payload['removed'] as $tag) {

            $group                                 = strtolower($tag['group']);
            $messageData['changes'][$group]['was'] = $tag['name'];
        }

        return $messageData;
    }

    /**
     * Check that the issue is load.
     *
     * @return bool
     */
    protected function validateData()
    {
        // if issue closed and last issue not closed, then something is wrong skip
        if (!$this->issue->isOpen() && $this->latestMessage->event !== Queue::CLOSE_ISSUE) {
            return false;
        }

        return $this->issue;
class Author {
    private $name;

    public function __construct($name) {
        $this->name = $name;
    }

    public function getName() {
        return $this->name;
    }
}

abstract class Post {
    public function getAuthor() {
        return 'Johannes';
    }
}

class BlogPost extends Post {
    public function getAuthor() {
        return new Author('Johannes');
    }
}

class ForumPost extends Post { /* ... */ }

function my_function(Post $post) {
    echo strtoupper($post->getAuthor());
}
    }

    /**
     * Populate assigned relation in the current issue.
     *
     * @return void
     */
    protected function populateData()

    {
        $this->issue->setRelation('assigned', $this->getUserById($this->issue->assigned_to));
        $creator = $this->getUserById($this->issue->created_by);
        if ($creator) {
            $this->issue->setRelation('user', $creator);
        }
        $this->loadIssueCreatorToProjectUsers();
    }

    /**
     * Check if the latest message is about closing or reopening an issue.
     *
     * @return bool
     */
    public function isStatusMessage()
    {
        return ((!$this->issue->isOpen() && $this->latestMessage->event === Queue::CLOSE_ISSUE)
            || ($this->issue->isOpen() && $this->latestMessage->event === Queue::REOPEN_ISSUE));
    }

    /**
     * Process assign to user message. Send direct message to new and previous users and full subscribers.
     *
     * @return void
     */
    protected function processDirectMessages()
    {
        // Stop if issue is closed
        if (!$this->getIssue()->isOpen()) {
class A
{
    public function foo() { }
}

class B extends A
{
    public function bar() { }
}

/**
 * @param A|B $x
 */
function someFunction($x)
{
    $x->foo(); // This call is fine as the method exists in A and B.
    $x->bar(); // This method only exists in B and might cause an error.
}
            return;
        }

        // Fetch all of the assign issue changes
        $assignMessages = $this->allMessages->where('event', Queue::ASSIGN_ISSUE);

        // Skip if no changes
        if ($assignMessages->isEmpty()) {
            return;
        }

        // Fetch the latest assignee
        /** @var Queue $assignMessage */
        $assignMessage = $assignMessages->first();

        // Fetch the user details of the new assignee & previous assignee if this isn't new issue
        $assigns        = [];
        $assigns['new'] = (int) $assignMessage->getDataFromPayload('dirty.assigned_to');
        if (!$this->addMessage) {
            $previousAssignMessage = $assignMessages->last();
            $assigns['old']        = (int) $previousAssignMessage->getDataFromPayload('origin.assigned_to');
        }

        // Fetch users objects for old and new assignee
        /** @var \Illuminate\Database\Eloquent\Collection $assignObjects */
        $assignObjects = (new User())->whereIn('id', $assigns)->get();
class ParentClass {
    private $data = array();

    public function __call($method, array $args) {
        if (0 === strpos($method, 'get')) {
            return $this->data[strtolower(substr($method, 3))];
        }

        throw new \LogicException(sprintf('Unsupported method: %s', $method));
    }
}

/**
 * If this class knows which fields exist, you can specify the methods here:
 *
 * @method string getName()
 */
class SomeClass extends ParentClass { }

        // If for what ever reason the user does not exists, skip this change
        // or if there is only one user and is not matching the new assignee.
        // then skip this message
        if ($assignObjects->count() === 0
            || ($assignObjects->count() === 1 && (int) $assignObjects->first()->id !== $assigns['new'])
        ) {
            return;
        }

        // Get the object of the new assignee
        $assignTo = $assignObjects->where('id', $assigns['new'], false)->first();
        $users    = collect([$this->createProjectUserObject($assignTo)]);

        // Exclude the user from any other message for this issue
        $this->addToExcludeUsers($assignTo);

        // Data about new and previous assignee
        $extraMessageData = [
            'now' => $assignTo->fullname,
        ];

        // Make sure that the previous assignee was not the same as the new user
        if (array_key_exists('old', $assigns) && $assigns['old'] > 0 && $assigns['new'] !== $assigns['old']) {
            $previousAssign = $assignObjects->where('id', $assigns['old'], false)->first();
            if ($previousAssign) {
                $extraMessageData['was'] = $previousAssign->fullname;
                $users->push($this->createProjectUserObject($previousAssign));
            }
        }

        // Get message data needed for the message & send
        $messageData = $this->getMessageData($assignMessage, $extraMessageData);
        $this->sendMessages($users, $messageData);
    }

    /**
     * Create user project object for a user.
     *
     * @param User $user
     *
     * @return Project\User
     */
    protected function createProjectUserObject(User $user)
    {
        $userProject = new Project\User([
            'user_id'    => $user->id,
            'project_id' => $this->getProjectId(),
        ]);
        $userProject->setRelation('user', $user);
        $userProject->setRelation('project', $this->getProject());

        return $userProject;
    }

    /**
     * Get collection of tags or one by ID.
     *
     * @param null|int $tagId
     *
     * @return \Illuminate\Support\Collection|Tag
     */
    protected function getTags($tagId = null)
    {
        if (null === $this->tags) {
            $this->tags = collect([]);
        }

        // Load & extract tag by ID
        if (null !== $tagId) {
            $tag = $this->tags->where('id', $tagId, false)->first();
            if (!$tag) {
                $tag = (new Tag())->find($tagId);
class ParentClass {
    private $data = array();

    public function __call($method, array $args) {
        if (0 === strpos($method, 'get')) {
            return $this->data[strtolower(substr($method, 3))];
        }

        throw new \LogicException(sprintf('Unsupported method: %s', $method));
    }
}

/**
 * If this class knows which fields exist, you can specify the methods here:
 *
 * @method string getName()
 */
class SomeClass extends ParentClass { }
                $this->tags->push($tag);
            }

            return $tag;
        }

        return $this->tags;
    }

    /**
     * Whether or not the user wants to receive the message.
     *
     * @param Project\User $user
     * @param array        $data
     *
     * @return bool
     */
    protected function wantToReceiveMessage(Project\User $user, array $data)
    {
        $status = parent::wantToReceiveMessage($user, $data);

        if (!$status) {
            return false;
        }

        // Search for tag changes and verify if the user can receive messages
        foreach ($data['changes'] as $label => $change) {
            // Skip other changes that are not related to tag
            if (!in_array($label, Tag::getCoreGroups())) {
                continue;
            }

            // If the change was only remove a tag
            if (!array_key_exists('now', $change) && array_key_exists('was', $change)) {
                return true;
            }

            // If tag id not found
            if (!array_key_exists('id', $change)) {
                return true;
            }

            // Fetch tag details
            $tag = $this->getTags($change['id']);

            // Check if user allowed to receive the message
            if (!$tag->allowMessagesToUser($user->user)) {
<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}
                return false;
            }
        }

        return true;
    }
}


Push — develop ( 7a0090...cf7e5a )

SendMessages C

Complexity

Size/Duplication

Coupling/Cohesion

Test Coverage

Importance

16 Methods

How to fix Duplicated Code Complexity

Duplicated Code

Complex Class

Available Fixes

Available Fixes

satrun77 / tinyissue

Push — develop ( 7a0090...cf7e5a )

SendMessages C

Complexity

Size/Duplication

Coupling/Cohesion

Test Coverage

Importance

16 Methods

How to fix Duplicated Code Complexity

Duplicated Code

Complex Class

Available Fixes

Available Fixes

Duplication Side-by-Side

Filter issues like