UnifiedAccount - Code Metrics - Inspection of "Merge pull request #1192 from owncloud/speed-up-un..." - owncloud/mail - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( ff72bb...5db731 )

by Jan-Christoph

created 2015-12-31 01:37 UTC

UnifiedAccount A

↳ Parent: Project

Complexity

Total Complexity

Size/Duplication

Total Lines	212
Duplicated Lines	3.77 %

Coupling/Cohesion

Components	1
Dependencies	4

Test Coverage

Coverage

4.72%

Importance

Changes	3
Bugs	0	Features	1

Metric	Value
wmc	26
c	3
b	0
f	1
lcom	1
cbo	4
dl	8
loc	212
ccs	5
cts	106
cp	0.0472
rs	10

13 Methods

Rating	Name	Duplication	Size	Complexity
A	__construct()	0	5	1
A	getConfiguration()	0	5	1
A	getListArray()	0	10	1
B	buildInbox()	0	37	6
A	getMailbox()	0	3	1
A	getEmail()	0	15	4
A	sendMessage()	0	3	1
A	saveDraft()	0	3	1
A	deleteMessage()	0	8	1
B	getChangedMailboxes()	0	33	6
A	getInbox()	0	3	1
A	getId()	0	3	1
A	resolve()	8	8	1

How to fix Duplicated Code

<?php

namespace OCA\Mail\Service;

use OCP\IL10N;
use OCA\Mail\Model\IMessage;

class UnifiedAccount implements IAccount {

	const ID = -1;
	const INBOX_ID = 'all-inboxes';

	/** @var AccountService */
	private $accountService;

	/** @var string */
	private $userId;

	/** @var IL10N */
	private $l10n;

	/** @var Horde_Mail_Rfc822_List */
	private $email;

	/**
	 * @param AccountService $accountService
	 * @param string $userId
	 * @param IL10N $l10n
	 */
	public function __construct(AccountService $accountService, $userId, IL10N $l10n) {
		$this->accountService = $accountService;
		$this->userId = $userId;
		$this->l10n = $l10n;
	}

	/**
	 * @return array
	 */
	public function getConfiguration() {
		return [
			'accountId' => UnifiedAccount::ID,
		];
	}

	/**
	 * @return array
	 * TODO: function name is :hankey:
	 */
	public function getListArray() {
		$inbox = $this->buildInbox();
		return [
			'id'             => UnifiedAccount::ID,
			'email'          => '',
			'folders'        => [$inbox],
			'specialFolders' => [],
			'delimiter' => '.',
		];
	}

	private function buildInbox() {
		$displayName = (string)$this->l10n->t('All inboxes');

		$allAccounts = $this->accountService->findByUserId($this->userId);

		$uidValidity = [];
		$uidNext = [];
		$unseen = 0;

		foreach($allAccounts as $account) {
			/** @var IAccount $account */
			$inbox = $account->getInbox();
			if (is_null($inbox)) {
				continue;
			}

			$status = $inbox->getStatus();
			$unseen += isset($status['unseen']) ? $status['unseen'] : 0;
			$uidValidity[$account->getId()] = isset($status['uidvalidity']) ? $status['uidvalidity'] : 0;
			$uidNext[$account->getId()] = isset($status['uidnext']) ? $status['uidnext'] : 0;
		}

		return [
			'id' => base64_encode(self::INBOX_ID),
			'parent' => null,
			'name' => $displayName,
			'specialRole' => 'inbox',
			'unseen' => $unseen,
			'total' => 100,
			'isEmpty' => false,
			'accountId' => UnifiedAccount::ID,
			'noSelect' => false,
			'uidvalidity' => $uidValidity,
			'uidnext' => $uidNext,
			'delimiter' => '.'
		];
	}

	/**
	 * @param $folderId
	 * @return IMailBox
	 */
	public function getMailbox($folderId) {
		return new UnifiedMailbox($this->accountService, $this->userId);
	}

	/**
	 * @return string
	 */
	public function getEmail() {
		if ($this->email === null) {
			$allAccounts = $this->accountService->findByUserId($this->userId);
			$addressesList = new \Horde_Mail_Rfc822_List();
			foreach ($allAccounts as $account) {
				$inbox = $account->getInbox();
				if (is_null($inbox)) {
					continue;
				}
				$addressesList->add($account->getEmail());
			}
			$this->email = $addressesList;

		}
		return $this->email;
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());
}
	}

	/**
	 * @param IMessage $message
	 * @param int|null $draftUID
	 */
	public function sendMessage(IMessage $message, $draftUID) {
		throw new Exception('Not implemented');
	}

	/**
	 * @param IMessage $message
	 * @param int|null $previousUID
	 * @return int
	 */
	public function saveDraft(IMessage $message, $previousUID) {
		throw new Exception('Not implemented');
	}

	/**
	 * @param string $folderId
	 * @param string $messageId
	 */
	public function deleteMessage($folderId, $messageId) {
		$data = json_decode(base64_decode($messageId), true);
		$account = $this->accountService->find($this->userId, $data[0]);
		$inbox = $account->getInbox();
		$messageId = $data[1];

		$account->deleteMessage($inbox->getFolderId(), $messageId);
	}

	/**
	 * @param string[] $query
	 * @return array
	 */
	public function getChangedMailboxes($query) {
		$accounts = $this->accountService->findByUserId($this->userId);
		$changedBoxes = [];

		foreach($accounts as $account) {
			/** @var IAccount $account */
			if ($account->getId() === UnifiedAccount::ID) {
				continue;
			}
			$inbox = $account->getInbox();
			$inboxName = $inbox->getFolderId();
			$changes = $account->getChangedMailboxes([$inboxName => [
function acceptsInteger($int) { }

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

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
				'uidvalidity' => $query[self::INBOX_ID]['uidvalidity'][$account->getId()],
				'uidnext' => $query[self::INBOX_ID]['uidnext'][$account->getId()],
			]]);
			if (!isset($changes[$inboxName])) {
				continue;
			}
			if (!isset($changedBoxes[self::INBOX_ID])) {
				$changedBoxes[self::INBOX_ID] = $this->buildInbox();
				$changedBoxes[self::INBOX_ID]['messages'] = [];
				$changedBoxes[self::INBOX_ID]['newUnReadCounter'] = 0;
			}
			// Create special unified inbox message IDs
			foreach ($changes[$inboxName]['messages'] as &$message) {
				$id = base64_encode(json_encode([$account->getId(), $message['id']]));
				$message['id'] = $id;
			}
			$changedBoxes[self::INBOX_ID]['messages'] = array_merge($changedBoxes[self::INBOX_ID]['messages'], $changes[$inboxName]['messages']);
			$changedBoxes[self::INBOX_ID]['newUnReadCounter'] += $changes[$inboxName]['newUnReadCounter'];
		}
		return $changedBoxes;
	}

	/**
	 * @return IMailBox
	 */
	public function getInbox() {
		return null;
	}

	/**
	 * @return int
	 */
	public function getId() {
		return UnifiedAccount::ID;
	}

	/**
	 * @param $messageId
	 * @return array
	 */
	public function resolve($messageId) {

		$data = json_decode(base64_decode($messageId), true);
		$account = $this->accountService->find($this->userId, $data[0]);
		$inbox = $account->getInbox();
		$messageId = $data[1];

		return [$account, base64_encode($inbox->getFolderId()), $messageId];
	}
}


1			<?php
2
3			namespace OCA\Mail\Service;
4
5			use OCP\IL10N;
6			use OCA\Mail\Model\IMessage;
7
8			class UnifiedAccount implements IAccount {
9
10			const ID = -1;
11			const INBOX_ID = 'all-inboxes';
12
13			/** @var AccountService */
14			private $accountService;
15
16			/** @var string */
17			private $userId;
18
19			/** @var IL10N */
20			private $l10n;
21
22			/** @var Horde_Mail_Rfc822_List */
23			private $email;
24
25			/**
26			* @param AccountService $accountService
27			* @param string $userId
28			* @param IL10N $l10n
29			*/
30	1		public function __construct(AccountService $accountService, $userId, IL10N $l10n) {
31	1		$this->accountService = $accountService;
32	1		$this->userId = $userId;
33	1		$this->l10n = $l10n;
34	1		}
35
36			/**
37			* @return array
38			*/
39			public function getConfiguration() {
40			return [
41			'accountId' => UnifiedAccount::ID,
42			];
43			}
44
45			/**
46			* @return array
47			* TODO: function name is :hankey:
48			*/
49			public function getListArray() {
50			$inbox = $this->buildInbox();
51			return [
52			'id' => UnifiedAccount::ID,
53			'email' => '',
54			'folders' => [$inbox],
55			'specialFolders' => [],
56			'delimiter' => '.',
57			];
58			}
59
60			private function buildInbox() {
61			$displayName = (string)$this->l10n->t('All inboxes');
62
63			$allAccounts = $this->accountService->findByUserId($this->userId);
64
65			$uidValidity = [];
66			$uidNext = [];
67			$unseen = 0;
68
69			foreach($allAccounts as $account) {
70			/** @var IAccount $account */
71			$inbox = $account->getInbox();
72			if (is_null($inbox)) {
73			continue;
74			}
75
76			$status = $inbox->getStatus();
77			$unseen += isset($status['unseen']) ? $status['unseen'] : 0;
78			$uidValidity[$account->getId()] = isset($status['uidvalidity']) ? $status['uidvalidity'] : 0;
79			$uidNext[$account->getId()] = isset($status['uidnext']) ? $status['uidnext'] : 0;
80			}
81
82			return [
83			'id' => base64_encode(self::INBOX_ID),
84			'parent' => null,
85			'name' => $displayName,
86			'specialRole' => 'inbox',
87			'unseen' => $unseen,
88			'total' => 100,
89			'isEmpty' => false,
90			'accountId' => UnifiedAccount::ID,
91			'noSelect' => false,
92			'uidvalidity' => $uidValidity,
93			'uidnext' => $uidNext,
94			'delimiter' => '.'
95			];
96			}
97
98			/**
99			* @param $folderId
100			* @return IMailBox
101			*/
102			public function getMailbox($folderId) {
103			return new UnifiedMailbox($this->accountService, $this->userId);
104			}
105
106			/**
107			* @return string
108			*/
109			public function getEmail() {
110			if ($this->email === null) {
111			$allAccounts = $this->accountService->findByUserId($this->userId);
112			$addressesList = new \Horde_Mail_Rfc822_List();
113			foreach ($allAccounts as $account) {
114			$inbox = $account->getInbox();
115			if (is_null($inbox)) {
116			continue;
117			}
118			$addressesList->add($account->getEmail());
119			}
120			$this->email = $addressesList;
			0 ignored issues – show Documentation Bug introduced 2015-12-29 19:54 UTC by Report Bug Copy Issue Report It seems like `$addressesList` of type `object<Horde_Mail_Rfc822_List>` is incompatible with the declared type `object<OCA\Mail\Service\Horde_Mail_Rfc822_List>` of property `$email`. Our type inference engine has found an assignment to a property that is incompatible with the declared type of that property. Either this assignment is in error or the assigned type should be added to the documentation/type hint for that property.. Loading history...
121			}
122			return $this->email;
			0 ignored issues – show Bug Best Practice introduced 2015-12-29 19:54 UTC by Report Bug Copy Issue Report The return type of `return $this->email;` (`Horde_Mail_Rfc822_List\|O...\Horde_Mail_Rfc822_List`) is incompatible with the return type declared by the interface `OCA\Mail\Service\IAccount::getEmail` of type `string`. If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design. Let’s take a look at an example: 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()); } Our function `my_function` expects a `Post` object, and outputs the author of the post. The base class `Post` returns a simple string and outputting a simple string will work just fine. However, the child class `BlogPost` which is a sub-type of `Post` instead decided to return an `object`, and is therefore violating the SOLID principles. If a `BlogPost` were passed to `my_function`, PHP would not complain, but ultimately fail when executing the `strtoupper` call in its body. Loading history...
123			}
124
125			/**
126			* @param IMessage $message
127			* @param int\|null $draftUID
128			*/
129			public function sendMessage(IMessage $message, $draftUID) {
130			throw new Exception('Not implemented');
131			}
132
133			/**
134			* @param IMessage $message
135			* @param int\|null $previousUID
136			* @return int
137			*/
138			public function saveDraft(IMessage $message, $previousUID) {
139			throw new Exception('Not implemented');
140			}
141
142			/**
143			* @param string $folderId
144			* @param string $messageId
145			*/
146			public function deleteMessage($folderId, $messageId) {
147			$data = json_decode(base64_decode($messageId), true);
148			$account = $this->accountService->find($this->userId, $data[0]);
149			$inbox = $account->getInbox();
150			$messageId = $data[1];
151
152			$account->deleteMessage($inbox->getFolderId(), $messageId);
153			}
154
155			/**
156			* @param string[] $query
157			* @return array
158			*/
159			public function getChangedMailboxes($query) {
160			$accounts = $this->accountService->findByUserId($this->userId);
161			$changedBoxes = [];
162
163			foreach($accounts as $account) {
164			/** @var IAccount $account */
165			if ($account->getId() === UnifiedAccount::ID) {
166			continue;
167			}
168			$inbox = $account->getInbox();
169			$inboxName = $inbox->getFolderId();
170			$changes = $account->getChangedMailboxes([$inboxName => [
			0 ignored issues – show Documentation introduced 2015-11-21 15:04 UTC by Report Bug Copy Issue Report `array($inboxName => arra...'][$account->getId()]))` is of type `array<string,array<strin...","uidnext":"string"}>>`, but the function expects a `array<integer,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...
171			'uidvalidity' => $query[self::INBOX_ID]['uidvalidity'][$account->getId()],
172			'uidnext' => $query[self::INBOX_ID]['uidnext'][$account->getId()],
173			]]);
174			if (!isset($changes[$inboxName])) {
175			continue;
176			}
177			if (!isset($changedBoxes[self::INBOX_ID])) {
178			$changedBoxes[self::INBOX_ID] = $this->buildInbox();
179			$changedBoxes[self::INBOX_ID]['messages'] = [];
180			$changedBoxes[self::INBOX_ID]['newUnReadCounter'] = 0;
181			}
182			// Create special unified inbox message IDs
183			foreach ($changes[$inboxName]['messages'] as &$message) {
184			$id = base64_encode(json_encode([$account->getId(), $message['id']]));
185			$message['id'] = $id;
186			}
187			$changedBoxes[self::INBOX_ID]['messages'] = array_merge($changedBoxes[self::INBOX_ID]['messages'], $changes[$inboxName]['messages']);
188			$changedBoxes[self::INBOX_ID]['newUnReadCounter'] += $changes[$inboxName]['newUnReadCounter'];
189			}
190			return $changedBoxes;
191			}
192
193			/**
194			* @return IMailBox
195			*/
196			public function getInbox() {
197			return null;
198			}
199
200			/**
201			* @return int
202			*/
203			public function getId() {
204			return UnifiedAccount::ID;
205			}
206
207			/**
208			* @param $messageId
209			* @return array
210			*/
211		View Code Duplication	public function resolve($messageId) {
			0 ignored issues – show Duplication introduced 2015-11-21 15:04 UTC by Report Bug Copy Issue Report This method seems to be duplicated in your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history...
212			$data = json_decode(base64_decode($messageId), true);
213			$account = $this->accountService->find($this->userId, $data[0]);
214			$inbox = $account->getInbox();
215			$messageId = $data[1];
216
217			return [$account, base64_encode($inbox->getFolderId()), $messageId];
218			}
219			}
220

owncloud / mail

Push — master ( ff72bb...5db731 )

UnifiedAccount A

Complexity

Size/Duplication

Coupling/Cohesion

Test Coverage

Importance

13 Methods

How to fix Duplicated Code

Duplicated Code

Duplication Side-by-Side

Filter issues like