StatsModel::getItems() - Code Metrics - Inspection of "We need single triggers" - joomla/statistics-server - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — mysql_improvements ( c32a38...450d27 )

by George

created 2016-05-02 19:39 UTC

StatsModel::getItems() C

↳ Parent: StatsModel

Complexity

Conditions	9
Paths	9

Size

Total Lines	69
Code Lines	39

Duplication

Lines	0
Ratio	0 %

Code Coverage

Tests	37
CRAP Score	9.0341

Importance

Changes	12
Bugs	0	Features	1

Metric	Value
c	12
b	0
f	1
dl	0
loc	69
ccs	37
cts	40
cp	0.925
rs	6.2192
cc	9
eloc	39
nc	9
nop	1
crap	9.0341

How to fix Long Method

<?php

namespace Stats\Models;

use Joomla\Database\Query\LimitableInterface;
use Joomla\Model\AbstractDatabaseModel;

/**
 * Statistics database model
 *
 * @since  1.0
 */
class StatsModel extends AbstractDatabaseModel
{
	/**
	 * The query batch size
	 *
	 * @var    integer
	 * @since  1.0
	 */
	private $batchSize = 25000;

	/**
	 * Loads the statistics data from the database.
	 *
	 * @param   string  $column  A single column to filter on
	 *
	 * @return  \array[]  Array of data arrays.
	 *
	 * @since   1.0
	 * @throws  \InvalidArgumentException
	 */
	public function getItems($column = null)
	{
		$db = $this->getDb();

		// Validate the requested column is actually in the table
		if ($column !== null)
		{
			switch ($column)
			{
				case 'php_version':
				case 'db_version':
				case 'db_type':
				case 'cms_version':
				case 'server_os':
					return $db->setQuery(
						$db->getQuery(true)
							->select('*')
							->from('#__jstats_counter_' . $column)
					)->loadAssocList($column);
					break;
switch ($x) {
    case 1:
        return 'foo';
        break; // This break is not necessary and can be left off.
}

				default:
					throw new \InvalidArgumentException('An invalid data source was requested.', 404);
			}
		}

		// If fetching all data from the table, we need to break this down a fair bit otherwise we're going to run out of memory
		$totalRecords = $db->setQuery(
			$db->getQuery(true)
				->select('COUNT(unique_id)')
				->from('#__jstats')
		)->loadResult();

		$return = [];

		$query = $db->getQuery(true)
			->select(['php_version', 'db_type', 'db_version', 'cms_version', 'server_os'])
			->from('#__jstats')
			->group('unique_id');

		$limitable = $query instanceof LimitableInterface;

		// We can't have this as a single array, we run out of memory... This is gonna get interesting...
		for ($offset = 0; $offset < $totalRecords; $offset + $this->batchSize)
		{
			if ($limitable)
			{
				$query->setLimit($this->batchSize, $offset);

				$db->setQuery($query);
			}
			else
			{
				$db->setQuery($query, $offset, $this->batchSize);
			}

			$return[] = $db->loadAssocList();

			$offset += $this->batchSize;
		}

		// Disconnect the DB to free some memory
		$db->disconnect();

		// And unset some variables
		unset($db, $query, $offset, $totalRecords);

		return $return;
	}

	/**
	 * Saves the given data.
	 *
	 * @param   \stdClass  $data  Data object to save.
	 *
	 * @return  void
	 *
	 * @since   1.0
	 */
	public function save($data)
	{
		$db = $this->getDb();

		// Set the modified date of the record
		$data->modified = (new \DateTime('now', new \DateTimeZone('UTC')))->format($db->getDateFormat());

		// Check if a row exists for this unique ID and update the existing record if so
		$recordExists = $db->setQuery(
			$db->getQuery(true)
				->select('unique_id')
				->from('#__jstats')
				->where('unique_id = ' . $db->quote($data->unique_id))
		)->loadResult();

		if ($recordExists)
		{
			$db->updateObject('#__jstats', $data, ['unique_id']);
		}
		else
		{
			$db->insertObject('#__jstats', $data, ['unique_id']);
function acceptsInteger($int) { }

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

// Instead of
acceptsInteger($x);

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


1		<?php
2
3		namespace Stats\Models;
4
5		use Joomla\Database\Query\LimitableInterface;
6		use Joomla\Model\AbstractDatabaseModel;
7
8		/**
9		* Statistics database model
10		*
11		* @since 1.0
12		*/
13		class StatsModel extends AbstractDatabaseModel
14		{
15		/**
16		* The query batch size
17		*
18		* @var integer
19		* @since 1.0
20		*/
21		private $batchSize = 25000;
22
23		/**
24		* Loads the statistics data from the database.
25		*
26		* @param string $column A single column to filter on
27		*
28		* @return \array[] Array of data arrays.
29		*
30		* @since 1.0
31		* @throws \InvalidArgumentException
32		*/
33	3	public function getItems($column = null)
34		{
35	3	$db = $this->getDb();
36
37		// Validate the requested column is actually in the table
38	3	if ($column !== null)
39	3	{
40		switch ($column)
41		{
42	2	case 'php_version':
43	2	case 'db_version':
44	2	case 'db_type':
45	2	case 'cms_version':
46	2	case 'server_os':
47	1	return $db->setQuery(
48	1	$db->getQuery(true)
49	1	->select('*')
50	1	->from('#__jstats_counter_' . $column)
51	1	)->loadAssocList($column);
52		break;
		0 ignored issues – show Unused Code introduced 2016-05-02 19:41 UTC by Report Bug Copy Issue Report `break` is not strictly necessary here and could be removed. The `break` statement is not necessary if it is preceded for example by a `return` statement: switch ($x) { case 1: return 'foo'; break; // This break is not necessary and can be left off. } If you would like to keep this construct to be consistent with other `case` statements, you can safely mark this issue as a false-positive. Loading history...
53
54	1	default:
55	1	throw new \InvalidArgumentException('An invalid data source was requested.', 404);
56	1	}
57		}
58
59		// If fetching all data from the table, we need to break this down a fair bit otherwise we're going to run out of memory
60	1	$totalRecords = $db->setQuery(
61	1	$db->getQuery(true)
62	1	->select('COUNT(unique_id)')
63	1	->from('#__jstats')
64	1	)->loadResult();
65
66	1	$return = [];
67
68	1	$query = $db->getQuery(true)
69	1	->select(['php_version', 'db_type', 'db_version', 'cms_version', 'server_os'])
70	1	->from('#__jstats')
71	1	->group('unique_id');
72
73	1	$limitable = $query instanceof LimitableInterface;
74
75		// We can't have this as a single array, we run out of memory... This is gonna get interesting...
76	1	for ($offset = 0; $offset < $totalRecords; $offset + $this->batchSize)
77		{
78		if ($limitable)
79	1	{
80		$query->setLimit($this->batchSize, $offset);
81
82		$db->setQuery($query);
83		}
84		else
85		{
86	1	$db->setQuery($query, $offset, $this->batchSize);
87		}
88
89	1	$return[] = $db->loadAssocList();
90
91	1	$offset += $this->batchSize;
92	1	}
93
94		// Disconnect the DB to free some memory
95	1	$db->disconnect();
96
97		// And unset some variables
98	1	unset($db, $query, $offset, $totalRecords);
99
100	1	return $return;
101		}
102
103		/**
104		* Saves the given data.
105		*
106		* @param \stdClass $data Data object to save.
107		*
108		* @return void
109		*
110		* @since 1.0
111		*/
112	2	public function save($data)
113		{
114	2	$db = $this->getDb();
115
116		// Set the modified date of the record
117	2	$data->modified = (new \DateTime('now', new \DateTimeZone('UTC')))->format($db->getDateFormat());
118
119		// Check if a row exists for this unique ID and update the existing record if so
120	2	$recordExists = $db->setQuery(
121	2	$db->getQuery(true)
122	2	->select('unique_id')
123	2	->from('#__jstats')
124	2	->where('unique_id = ' . $db->quote($data->unique_id))
125	2	)->loadResult();
126
127		if ($recordExists)
128	2	{
129	1	$db->updateObject('#__jstats', $data, ['unique_id']);
130	1	}
131		else
132		{
133	1	$db->insertObject('#__jstats', $data, ['unique_id']);
		0 ignored issues – show Documentation introduced 2016-01-02 17:06 UTC by Report Bug Copy Issue Report `array('unique_id')` is of type `array<integer,string,{"0":"string"}>`, but the function expects a `string\|null`. 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...
134		}
135	2	}
136		}
137

joomla / statistics-server

Push — mysql_improvements ( c32a38...450d27 )

StatsModel::getItems() C

Complexity

Size

Duplication

Code Coverage

Importance

How to fix Long Method

Long Method

Duplication Side-by-Side

Filter issues like