StatsJsonView - Code Metrics - Inspection of "Try this another way" - joomla/statistics-server - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( 362b12...9f5558 )

by Michael

created 2016-04-02 14:52 UTC

StatsJsonView B

↳ Parent: Project

Complexity

Total Complexity

Size/Duplication

Total Lines	329
Duplicated Lines	7.9 %

Coupling/Cohesion

Components	1
Dependencies	2

Test Coverage

Coverage

95.21%

Importance

Changes	13
Bugs	0	Features	0

Metric	Value
wmc	42
c	13
b	0
f	0
lcom	1
cbo	2
dl	26
loc	329
ccs	139
cts	146
cp	0.9521
rs	8.295

6 Methods

Rating	Name	Duplication	Size	Complexity
A	isAuthorizedRaw()	0	4	1
C	render()	9	74	11
A	setSource()	0	4	1
B	buildResponseData()	0	25	5
D	processSingleSource()	9	45	9
D	sanitizeData()	8	84	15

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 StatsJsonView 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 StatsJsonView, and based on these observations, apply Extract Interface, too.

<?php

namespace Stats\Views\Stats;

use Joomla\View\BaseJsonView;

/**
 * JSON response for requesting the stats data.
 *
 * @property-read  \Stats\Models\StatsModel  $model  The model object.
 *
 * @since          1.0
 */
class StatsJsonView extends BaseJsonView
{
	/**
	 * Flag if the response should return the raw data.
	 *
	 * @var    boolean
	 * @since  1.0
	 */
	private $authorizedRaw = false;

	/**
	 * Array holding the valid data sources.
	 *
	 * @var    array
	 * @since  1.0
	 */
	private $dataSources = ['php_version', 'db_type', 'db_version', 'cms_version', 'server_os'];

	/**
	 * The data source to return.
	 *
	 * @var    string
	 * @since  1.0
	 */
	private $source;

	/**
	 * Count of the number of items.
	 *
	 * @var    integer
	 * @since  1.0
	 */
	private $totalItems = 0;

	/**
	 * Set whether the raw data should be returned.
	 *
	 * @param   boolean  $authorizedRaw  Flag if the response should return the raw data.
	 *
	 * @return  void
	 *
	 * @since   1.0
	 */
	public function isAuthorizedRaw($authorizedRaw)
	{
		$this->authorizedRaw = $authorizedRaw;
	}

	/**
	 * Method to render the view.
	 *
	 * @return  string  The rendered view.
	 *
	 * @since   1.0
	 * @throws  \InvalidArgumentException
	 */
	public function render()
	{
		$items = $this->model->getItems($this->source);

		$php_version = [];
		$db_type     = [];
		$db_version  = [];
		$cms_version = [];
		$server_os   = [];

		if ($this->source)
		{
			return $this->processSingleSource($items);
		}

		// If we have the entire database, we have to loop within each group to put it all together
		for ($i = 0; $i < count($items); $i++)
for ($i=0; $i<count($array); $i++) { // calls count() on each iteration
}

// Better
for ($i=0, $c=count($array); $i<$c; $i++) { // calls count() just once
}
		{
			$group = $items[$i];

			$this->totalItems += count($group);

			for ($k = 0; $k < count($group); $k++)
for ($i=0; $i<count($array); $i++) { // calls count() on each iteration
}

// Better
for ($i=0, $c=count($array); $i<$c; $i++) { // calls count() just once
}
			{
				$item = $group[$k];

				foreach ($this->dataSources as $source)
				{
					if (isset($item->$source) && !is_null($item->$source))
					{
						// Special case, if the server is empty then change the key to "unknown"
						if ($source === 'server_os' && empty($item->$source))
						{
							if (!isset(${$source}['unknown']))
							{
								${$source}['unknown'] = 0;
							}

							${$source}['unknown']++;
						}
						else

						{
							if (!isset(${$source}[$item->$source]))
							{
								${$source}[$item->$source] = 0;
							}

							${$source}[$item->$source]++;
						}
					}
				}

				unset($group[$k]);
			}
		}

		unset($items);

		$data = [
			'php_version' => $php_version,
			'db_type'     => $db_type,
			'db_version'  => $db_version,
			'cms_version' => $cms_version,
			'server_os'   => $server_os,
		];

		$responseData = $this->buildResponseData($data);

		$responseData['total'] = $this->totalItems;

		$this->addData('data', $responseData);

		return parent::render();
	}

	/**
	 * Set the data source.
	 *
	 * @param   string  $source  Data source to return.
	 *
	 * @return  void
	 *
	 * @since   1.0
	 */
	public function setSource($source)
	{
		$this->source = $source;
	}

	/**
	 * Process the raw data into the response data format.
	 *
	 * @param   array  $data  The raw data array.
	 *
	 * @return  array
	 *
	 * @since   1.0
	 */
	private function buildResponseData(array $data)
	{
		$responseData = [];

		foreach ($data as $key => $value)
		{
			foreach ($value as $name => $count)
			{
				if ($name)
				{
					$responseData[$key][] = [
						'name'  => $name,
						'count' => $count
					];
				}
			}
		}

		if (!$this->authorizedRaw)
		{
			$responseData = $this->sanitizeData($responseData);
		}

		return $responseData;
	}

	/**
	 * Process the response for a single data source.
	 *
	 * @param   array  $items  The source items to process.
	 *
	 * @return  string  The rendered view.
	 *
	 * @since   1.0
	 */
	private function processSingleSource(array $items)
	{
		$data = [
			${$this->source} = [],
		];

		$this->totalItems = count($items);

		foreach ($items as $item)
		{
			foreach ($this->dataSources as $source)
			{
				if (isset($item->$source) && !is_null($item->$source))
				{
					// Special case, if the server is empty then change the key to "unknown"
					if ($source === 'server_os' && empty($item->$source))
					{
						if (!isset($data[$source]['unknown']))
						{
							$data[$source]['unknown'] = 0;
						}

						$data[$source]['unknown']++;
					}
					else

					{
						if (!isset($data[$source][$item->$source]))
						{
							$data[$source][$item->$source] = 0;
						}

						$data[$source][$item->$source]++;
					}
				}
			}
		}

		$responseData = $this->buildResponseData($data);

		$responseData['total'] = $this->totalItems;

		$this->addData('data', $responseData);

		return parent::render();
class Daddy
{
    protected function getFirstName()
    {
        return "Eidur";
    }

    protected function getSurName()
    {
        return "Gudjohnsen";
    }
}

class Son
{
    public function getFirstName()
    {
        return parent::getSurname();
    }
}
	}

	/**
	 * Sanitize the response data into summarized groups.
	 *
	 * @param   array  $responseData  The response data to sanitize.
	 *
	 * @return  array
	 *
	 * @since   1.0
	 */
	private function sanitizeData(array $responseData)
	{
		foreach ($responseData as $key => $dataGroup)
		{
			switch ($key)
			{
				case 'php_version':
				case 'db_version':
					// We're going to group by minor version branch here and convert to a percentage
					$counts = [];

					foreach ($dataGroup as $row)
					{
						$exploded = explode('.', $row['name']);
						$version  = $exploded[0] . '.' . (isset($exploded[1]) ? $exploded[1] : '0');

						// If the container does not exist, add it
						if (!isset($counts[$version]))
						{
							$counts[$version] = 0;
						}

						$counts[$version] += $row['count'];
					}

					$sanitizedData = [];

					foreach ($counts as $version => $count)

					{
						$sanitizedData[$version] = round(($count / $this->totalItems) * 100, 2);
					}

					$responseData[$key] = $sanitizedData;

					break;

				case 'server_os':
					// We're going to group by operating system here
					$counts = [];

					foreach ($dataGroup as $row)
					{
						$fullOs = explode(' ', $row['name']);
						$os     = $fullOs[0];

						// If the container does not exist, add it
						if (!isset($counts[$os]))
						{
							$counts[$os] = 0;
						}

						$counts[$os] += $row['count'];
					}

					$sanitizedData = [];

					foreach ($counts as $os => $count)

					{
						$sanitizedData[$os] = round(($count / $this->totalItems) * 100, 2);
					}

					$responseData[$key] = $sanitizedData;

					break;

				case 'db_type':
				case 'cms_version':
				default:
					// For now, group by the object name and figure out the percentages
					$sanitizedData = [];

					foreach ($dataGroup as $row)
					{
						$sanitizedData[$row['name']] = round(($row['count'] / $this->totalItems) * 100, 2);
					}

					$responseData[$key] = $sanitizedData;

					break;
			}
		}

		return $responseData;
	}
}


1			<?php
2
3			namespace Stats\Views\Stats;
4
5			use Joomla\View\BaseJsonView;
6
7			/**
8			* JSON response for requesting the stats data.
9			*
10			* @property-read \Stats\Models\StatsModel $model The model object.
11			*
12			* @since 1.0
13			*/
14			class StatsJsonView extends BaseJsonView
15			{
16			/**
17			* Flag if the response should return the raw data.
18			*
19			* @var boolean
20			* @since 1.0
21			*/
22			private $authorizedRaw = false;
23
24			/**
25			* Array holding the valid data sources.
26			*
27			* @var array
28			* @since 1.0
29			*/
30			private $dataSources = ['php_version', 'db_type', 'db_version', 'cms_version', 'server_os'];
31
32			/**
33			* The data source to return.
34			*
35			* @var string
36			* @since 1.0
37			*/
38			private $source;
39
40			/**
41			* Count of the number of items.
42			*
43			* @var integer
44			* @since 1.0
45			*/
46			private $totalItems = 0;
47
48			/**
49			* Set whether the raw data should be returned.
50			*
51			* @param boolean $authorizedRaw Flag if the response should return the raw data.
52			*
53			* @return void
54			*
55			* @since 1.0
56			*/
57	1		public function isAuthorizedRaw($authorizedRaw)
58			{
59	1		$this->authorizedRaw = $authorizedRaw;
60	1		}
61
62			/**
63			* Method to render the view.
64			*
65			* @return string The rendered view.
66			*
67			* @since 1.0
68			* @throws \InvalidArgumentException
69			*/
70	4		public function render()
71			{
72	4		$items = $this->model->getItems($this->source);
73
74	4		$php_version = [];
75	4		$db_type = [];
76	4		$db_version = [];
77	4		$cms_version = [];
78	4		$server_os = [];
79
80	4		if ($this->source)
81	4		{
82	2		return $this->processSingleSource($items);
83			}
84
85			// If we have the entire database, we have to loop within each group to put it all together
86	2		for ($i = 0; $i < count($items); $i++)
			0 ignored issues – show Performance Best Practice introduced 2016-04-02 14:50 UTC by Report Bug Copy Issue Report It seems like you are calling the size function `count()` as part of the test condition. You might want to compute the size beforehand, and not on each iteration. If the size of the collection does not change during the iteration, it is generally a good practice to compute it beforehand, and not on each iteration: for ($i=0; $i<count($array); $i++) { // calls count() on each iteration } // Better for ($i=0, $c=count($array); $i<$c; $i++) { // calls count() just once } Loading history...
87			{
88	2		$group = $items[$i];
89
90	2		$this->totalItems += count($group);
91
92	2		for ($k = 0; $k < count($group); $k++)
			0 ignored issues – show Performance Best Practice introduced 2016-04-02 14:54 UTC by Report Bug Copy Issue Report It seems like you are calling the size function `count()` as part of the test condition. You might want to compute the size beforehand, and not on each iteration. If the size of the collection does not change during the iteration, it is generally a good practice to compute it beforehand, and not on each iteration: for ($i=0; $i<count($array); $i++) { // calls count() on each iteration } // Better for ($i=0, $c=count($array); $i<$c; $i++) { // calls count() just once } Loading history...
93			{
94	2		$item = $group[$k];
95
96	2		foreach ($this->dataSources as $source)
97			{
98	2		if (isset($item->$source) && !is_null($item->$source))
99	2		{
100			// Special case, if the server is empty then change the key to "unknown"
101	2		if ($source === 'server_os' && empty($item->$source))
102	2		{
103			if (!isset(${$source}['unknown']))
104			{
105			${$source}['unknown'] = 0;
106			}
107
108			${$source}['unknown']++;
109			}
110		View Code Duplication	else
			0 ignored issues – show Duplication introduced 2016-04-02 14:37 UTC by Report Bug Copy Issue Report This code seems to be duplicated across 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...
111			{
112	2		if (!isset(${$source}[$item->$source]))
113	2		{
114	2		${$source}[$item->$source] = 0;
115	2		}
116
117	2		${$source}[$item->$source]++;
118			}
119	2		}
120	2		}
121
122	2		unset($group[$k]);
123	2		}
124	2		}
125
126	2		unset($items);
127
128			$data = [
129	2		'php_version' => $php_version,
130	2		'db_type' => $db_type,
131	2		'db_version' => $db_version,
132	2		'cms_version' => $cms_version,
133	2		'server_os' => $server_os,
134	2		];
135
136	2		$responseData = $this->buildResponseData($data);
137
138	2		$responseData['total'] = $this->totalItems;
139
140	2		$this->addData('data', $responseData);
141
142	2		return parent::render();
143			}
144
145			/**
146			* Set the data source.
147			*
148			* @param string $source Data source to return.
149			*
150			* @return void
151			*
152			* @since 1.0
153			*/
154	1		public function setSource($source)
155			{
156	1		$this->source = $source;
157	1		}
158
159			/**
160			* Process the raw data into the response data format.
161			*
162			* @param array $data The raw data array.
163			*
164			* @return array
165			*
166			* @since 1.0
167			*/
168	4		private function buildResponseData(array $data)
169			{
170	4		$responseData = [];
171
172	4		foreach ($data as $key => $value)
173			{
174	4		foreach ($value as $name => $count)
175			{
176			if ($name)
177	4		{
178	4		$responseData[$key][] = [
179	4		'name' => $name,
180			'count' => $count
181	4		];
182	4		}
183	4		}
184	4		}
185
186	4		if (!$this->authorizedRaw)
187	4		{
188	3		$responseData = $this->sanitizeData($responseData);
189	3		}
190
191	4		return $responseData;
192			}
193
194			/**
195			* Process the response for a single data source.
196			*
197			* @param array $items The source items to process.
198			*
199			* @return string The rendered view.
200			*
201			* @since 1.0
202			*/
203	2		private function processSingleSource(array $items)
204			{
205			$data = [
206	2		${$this->source} = [],
207	2		];
208
209	2		$this->totalItems = count($items);
210
211	2		foreach ($items as $item)
212			{
213	2		foreach ($this->dataSources as $source)
214			{
215	2		if (isset($item->$source) && !is_null($item->$source))
216	2		{
217			// Special case, if the server is empty then change the key to "unknown"
218	2		if ($source === 'server_os' && empty($item->$source))
219	2		{
220	1		if (!isset($data[$source]['unknown']))
221	1		{
222	1		$data[$source]['unknown'] = 0;
223	1		}
224
225	1		$data[$source]['unknown']++;
226	1		}
227		View Code Duplication	else
			0 ignored issues – show Duplication introduced 2016-04-02 14:37 UTC by Report Bug Copy Issue Report This code seems to be duplicated across 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...
228			{
229	2		if (!isset($data[$source][$item->$source]))
230	2		{
231	2		$data[$source][$item->$source] = 0;
232	2		}
233
234	2		$data[$source][$item->$source]++;
235			}
236	2		}
237	2		}
238	2		}
239
240	2		$responseData = $this->buildResponseData($data);
241
242	2		$responseData['total'] = $this->totalItems;
243
244	2		$this->addData('data', $responseData);
245
246	2		return parent::render();
			0 ignored issues – show Comprehensibility Bug introduced 2016-04-02 14:37 UTC by Report Bug Copy Issue Report It seems like you call parent on a different method (`render()` instead of `processSingleSource()`). Are you sure this is correct? If so, you might want to change this to `$this->render()`. This check looks for a call to a parent method whose name is different than the method from which it is called. Consider the following code: class Daddy { protected function getFirstName() { return "Eidur"; } protected function getSurName() { return "Gudjohnsen"; } } class Son { public function getFirstName() { return parent::getSurname(); } } The `getFirstName()` method in the `Son` calls the wrong method in the parent class. Loading history...
247			}
248
249			/**
250			* Sanitize the response data into summarized groups.
251			*
252			* @param array $responseData The response data to sanitize.
253			*
254			* @return array
255			*
256			* @since 1.0
257			*/
258	3		private function sanitizeData(array $responseData)
259			{
260	3		foreach ($responseData as $key => $dataGroup)
261			{
262			switch ($key)
263			{
264	3		case 'php_version':
265	3		case 'db_version':
266			// We're going to group by minor version branch here and convert to a percentage
267	2		$counts = [];
268
269	2		foreach ($dataGroup as $row)
270			{
271	2		$exploded = explode('.', $row['name']);
272	2		$version = $exploded[0] . '.' . (isset($exploded[1]) ? $exploded[1] : '0');
273
274			// If the container does not exist, add it
275	2		if (!isset($counts[$version]))
276	2		{
277	2		$counts[$version] = 0;
278	2		}
279
280	2		$counts[$version] += $row['count'];
281	2		}
282
283	2		$sanitizedData = [];
284
285	2	View Code Duplication	foreach ($counts as $version => $count)
			0 ignored issues – show Duplication introduced 2016-01-02 17:06 UTC by Report Bug Copy Issue Report This code seems to be duplicated across 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...
286			{
287	2		$sanitizedData[$version] = round(($count / $this->totalItems) * 100, 2);
288	2		}
289
290	2		$responseData[$key] = $sanitizedData;
291
292	2		break;
293
294	2		case 'server_os':
295			// We're going to group by operating system here
296	2		$counts = [];
297
298	2		foreach ($dataGroup as $row)
299			{
300	2		$fullOs = explode(' ', $row['name']);
301	2		$os = $fullOs[0];
302
303			// If the container does not exist, add it
304	2		if (!isset($counts[$os]))
305	2		{
306	2		$counts[$os] = 0;
307	2		}
308
309	2		$counts[$os] += $row['count'];
310	2		}
311
312	2		$sanitizedData = [];
313
314	2	View Code Duplication	foreach ($counts as $os => $count)
			0 ignored issues – show Duplication introduced 2016-01-02 17:06 UTC by Report Bug Copy Issue Report This code seems to be duplicated across 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...
315			{
316	2		$sanitizedData[$os] = round(($count / $this->totalItems) * 100, 2);
317	2		}
318
319	2		$responseData[$key] = $sanitizedData;
320
321	2		break;
322
323	1		case 'db_type':
324	1		case 'cms_version':
325	1		default:
326			// For now, group by the object name and figure out the percentages
327	1		$sanitizedData = [];
328
329	1		foreach ($dataGroup as $row)
330			{
331	1		$sanitizedData[$row['name']] = round(($row['count'] / $this->totalItems) * 100, 2);
332	1		}
333
334	1		$responseData[$key] = $sanitizedData;
335
336	1		break;
337			}
338	3		}
339
340	3		return $responseData;
341			}
342			}
343

joomla / statistics-server