acp_module::main() - Code Metrics - Inspection of "Merge pull request #68 from paul999/remove_push" - paul999/ajax-shoutbox-ext - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — develop-stable ( 2a7b07...4a69f7 )

by Paul

created 2015-12-25 09:52 UTC

acp_module::main() F

↳ Parent: acp_module

Complexity

Conditions	22
Paths	448

Size

Total Lines	140
Code Lines	68

Duplication

Lines	0
Ratio	0 %

Importance

Changes	22
Bugs	8	Features	4

Metric	Value
c	22
b	8
f	4
dl	0
loc	140
rs	3.3431
cc	22
eloc	68
nc	448
nop	2

How to fix Long Method Complexity

<?php
/**
 *
 * Ajax Shoutbox extension for the phpBB Forum Software package.
 *
 * @copyright (c) 2014 Paul Sohier <http://www.ajax-shoutbox.com>
 * @license GNU General Public License, version 2 (GPL-2.0)
 *
 */

namespace paul999\ajaxshoutbox\acp;

class acp_module {
	/** @var  string */
	public $u_action;

	/** @var array  */
	public $new_config = array();

	/** @var   */
	public $page_title;

	/** @var   */
	public $tpl_name;

	function main($id, $mode)
// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

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

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
	{
		global $user, $template, $request;
function myFunction($a, $b) {
    // Do something
}
		global $config, $phpbb_dispatcher, $phpbb_log;
function myFunction($a, $b) {
    // Do something
}

		$user->add_lang_ext("paul999/ajaxshoutbox", "acp_ajax_shoutbox");
		$user->add_lang('acp/board');

		$submit = $request->is_set('submit');

		$form_key = 'acp_shoutbox';
		add_form_key($form_key);

		$display_vars = array(
			'title'	=> 'ACP_AJAXSHOUTBOX_SETTINGS',
			'vars'	=> array(
				'legend1'				=> 'ACP_AJAXSHOUTBOX_PRUNE',
				'ajaxshoutbox_enable_prune'			=> array('lang' => 'AJAXSHOUTBOX_ENABLE_PRUNE',			'validate' => 'bool',	'type' => 'radio:yes_no','explain' => false),
				'ajaxshoutbox_prune_days'			=> array('lang' => 'AJAXSHOUTBOX_PRUNE_DAYS',			'validate' => 'int',	'type' => 'number:1:9999','explain' => false, 'append' => ' ' . $user->lang['DAYS']),
				'legend2'				=> 'ACP_AJAXSHOUTBOX_SETTINGS',
				'ajaxshoutbox_date_format'      	=> array('lang' => 'AJAXSHOUTBOX_DEFAULT_DATE_FORMAT',	'validate' => 'string',	'type' => 'custom', 'method' => 'dateformat_select', 'explain' => true),

				'legend4'				=> 'ACP_SUBMIT_CHANGES',
			)
		);

		/**
		* Event to add and/or modify acp_board configurations
		*
		* @event paul999.ajaxshoutbox.shoutbox_config_edit_add
		* @var	array	display_vars	Array of config values to display and process
		* @var	string	mode			Mode of the config page we are displaying
		* @var	boolean	submit			Do we display the form or process the submission
		* @since 1.0.0-b2
		*/
		$vars = array('display_vars', 'mode', 'submit');
		extract($phpbb_dispatcher->trigger_event('paul999.ajaxshoutbox.shoutbox_config_edit_add', compact($vars)));

		$this->new_config = $config;
		// Copied from acp_board.php
		$cfg_array = (isset($_REQUEST['config'])) ? utf8_normalize_nfc($request->variable('config', array('' => ''), true)) : $this->new_config;
		$error = array();

		// We validate the complete config if wished
		validate_config_vars($display_vars['vars'], $cfg_array, $error);

		if ($submit && !check_form_key($form_key))
		{
			$error[] = $user->lang['FORM_INVALID'];
		}
		// Do not write values if there is an error
		if (sizeof($error))
		{
			$submit = false;
		}

		// We go through the display_vars to make sure no one is trying to set variables he/she is not allowed to...
		foreach ($display_vars['vars'] as $config_name => $null)
		{
			if (!isset($cfg_array[$config_name]) || strpos($config_name, 'legend') !== false || $config_name == 'ajaxshoutbox_validation_id' || $config_name == 'ajaxshoutbox_push_disabled')
			{
				continue;
			}

			$this->new_config[$config_name] = $config_value = $cfg_array[$config_name];

			if ($submit)
			{
				$config->set($config_name, $config_value);
			}
		}

		if ($submit)
		{
			$phpbb_log->add('admin', $user->data['user_id'], $user->ip, 'LOG_AJAX_SHOUTBOX_CONFIG_' . strtoupper($mode));

			$message = $user->lang('CONFIG_UPDATED');
			$message_type = E_USER_NOTICE;

			trigger_error($message . adm_back_link($this->u_action), $message_type);
		}

		$this->tpl_name = 'acp_board';
		$this->page_title = $display_vars['title'];

		$template->assign_vars(array(
			'L_TITLE'			=> $user->lang[$display_vars['title']],
			'L_TITLE_EXPLAIN'	=> $user->lang[$display_vars['title'] . '_EXPLAIN'],

			'S_ERROR'			=> (sizeof($error)) ? true : false,
			'ERROR_MSG'			=> implode('<br />', $error),

			'U_ACTION'			=> $this->u_action,
		));

		// Output relevant page
		foreach ($display_vars['vars'] as $config_key => $vars)
		{
			if (!is_array($vars) && strpos($config_key, 'legend') === false)
			{
				continue;
			}

			if (strpos($config_key, 'legend') !== false)
			{
				$template->assign_block_vars('options', array(
					'S_LEGEND'		=> true,
					'LEGEND'		=> (isset($user->lang[$vars])) ? $user->lang[$vars] : $vars)
				);

				continue;
			}

			$type = explode(':', $vars['type']);

			$l_explain = '';
			if ($vars['explain'] && isset($user->lang[$vars['lang'] . '_EXPLAIN']))
			{
				$l_explain =  $user->lang[$vars['lang'] . '_EXPLAIN'];
			}

			$content = build_cfg_template($type, $config_key, $this->new_config, $config_key, $vars);

			if (empty($content))
			{
				continue;
			}

			$template->assign_block_vars('options', array(
				'KEY'			=> $config_key,
				'TITLE'			=> (isset($user->lang[$vars['lang']])) ? $user->lang[$vars['lang']] : $vars['lang'],
				'S_EXPLAIN'		=> $vars['explain'],
				'TITLE_EXPLAIN'	=> $l_explain,
				'CONTENT'		=> $content,

			));

			unset($display_vars['vars'][$config_key]);
		}
	}

	/**
	 * Set the date format.
	 * @param $value
	 * @param $key
	 *
	 * @return string
	 */
	function dateformat_select($value, $key)

	{
		global $user, $config;
function myFunction($a, $b) {
    // Do something
}
		// Let the format_date function operate with the acp values
		$old_tz = $user->timezone;
		try
		{
			$user->timezone = new \DateTimeZone($config['board_timezone']);
		}
		catch (\Exception $e)
		{
			// If the board timezone is invalid, we just use the users timezone.
		}
		$dateformat_options = '';
		foreach ($user->lang['dateformats'] as $format => $null)
		{
			if (strpos($format, '|') === 0) // Skip relative formats!
			{
				continue;
			}

			$dateformat_options .= '<option value="' . $format . '"' . (($format == $value) ? ' selected="selected"' : '') . '>';
			$dateformat_options .= $user->format_date(time(), $format, false) . ((strpos($format, '|') !== false) ? $user->lang['VARIANT_DATE_SEPARATOR'] . $user->format_date(time(), $format, true) : '');
			$dateformat_options .= '</option>';
		}
		$dateformat_options .= '<option value="custom"';
		if (!isset($user->lang['dateformats'][$value]))
		{
			$dateformat_options .= ' selected="selected"';
		}
		$dateformat_options .= '>' . $user->lang['CUSTOM_DATEFORMAT'] . '</option>';
		// Reset users date options
		$user->timezone = $old_tz;
		return "<select name=\"dateoptions\" id=\"dateoptions\" onchange=\"if (this.value == 'custom') { document.getElementById('" . addslashes($key) . "').value = '" . addslashes($value) . "'; } else { document.getElementById('" . addslashes($key) . "').value = this.value; }\">$dateformat_options</select>
		<input type=\"text\" name=\"config[$key]\" id=\"$key\" value=\"$value\" maxlength=\"30\" />";
	}
}


Push — develop-stable ( 2a7b07...4a69f7 )

acp_module::main() F

Complexity

Size

Duplication

Importance

How to fix Long Method Complexity

Long Method

1. Pass all data via parameters

2. Create a class that maintains your state

1. Pass all data via parameters

2. Create a class that maintains your state

1. Pass all data via parameters

2. Create a class that maintains your state

1			<?php
2			/**
3			*
4			* Ajax Shoutbox extension for the phpBB Forum Software package.
5			*
6			* @copyright (c) 2014 Paul Sohier <http://www.ajax-shoutbox.com>
7			* @license GNU General Public License, version 2 (GPL-2.0)
8			*
9			*/
10
11			namespace paul999\ajaxshoutbox\acp;
12
13			class acp_module {
14			/** @var string */
15			public $u_action;
16
17			/** @var array */
18			public $new_config = array();
19
20			/** @var */
21			public $page_title;
22
23			/** @var */
24			public $tpl_name;
25
26			function main($id, $mode)
			0 ignored issues – show Best Practice introduced 2015-12-09 13:37 UTC by Report Bug Copy Issue Report It is generally recommended to explicitly declare the visibility for methods. Adding explicit visibility (`private`, `protected`, or `public`) is generally recommend to communicate to other developers how, and from where this method is intended to be used. Loading history... Coding Style introduced 2015-12-19 20:34 UTC by Report Bug Copy Issue Report `main` uses the super-global variable `$_REQUEST` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history...
27			{
28			global $user, $template, $request;
			0 ignored issues – show Compatibility Best Practice introduced 2015-12-19 20:34 UTC by Report Bug Copy Issue Report Use of `global` functionality is not recommended; it makes your code harder to test, and less reusable. Instead of relying on `global` state, we recommend one of these alternatives: 1. Pass all data via parameters function myFunction($a, $b) { // Do something } 2. Create a class that maintains your state class MyClass { private $a; private $b; public function __construct($a, $b) { $this->a = $a; $this->b = $b; } public function myFunction() { // Do something } } Loading history...
29			global $config, $phpbb_dispatcher, $phpbb_log;
			0 ignored issues – show Compatibility Best Practice introduced 2015-12-19 20:34 UTC by Report Bug Copy Issue Report Use of `global` functionality is not recommended; it makes your code harder to test, and less reusable. Instead of relying on `global` state, we recommend one of these alternatives: 1. Pass all data via parameters function myFunction($a, $b) { // Do something } 2. Create a class that maintains your state class MyClass { private $a; private $b; public function __construct($a, $b) { $this->a = $a; $this->b = $b; } public function myFunction() { // Do something } } Loading history...
30
31			$user->add_lang_ext("paul999/ajaxshoutbox", "acp_ajax_shoutbox");
32			$user->add_lang('acp/board');
33
34			$submit = $request->is_set('submit');
35
36			$form_key = 'acp_shoutbox';
37			add_form_key($form_key);
38
39			$display_vars = array(
40			'title' => 'ACP_AJAXSHOUTBOX_SETTINGS',
41			'vars' => array(
42			'legend1' => 'ACP_AJAXSHOUTBOX_PRUNE',
43			'ajaxshoutbox_enable_prune' => array('lang' => 'AJAXSHOUTBOX_ENABLE_PRUNE', 'validate' => 'bool', 'type' => 'radio:yes_no','explain' => false),
44			'ajaxshoutbox_prune_days' => array('lang' => 'AJAXSHOUTBOX_PRUNE_DAYS', 'validate' => 'int', 'type' => 'number:1:9999','explain' => false, 'append' => ' ' . $user->lang['DAYS']),
45			'legend2' => 'ACP_AJAXSHOUTBOX_SETTINGS',
46			'ajaxshoutbox_date_format' => array('lang' => 'AJAXSHOUTBOX_DEFAULT_DATE_FORMAT', 'validate' => 'string', 'type' => 'custom', 'method' => 'dateformat_select', 'explain' => true),
47
48			'legend4' => 'ACP_SUBMIT_CHANGES',
49			)
50			);
51
52			/**
53			* Event to add and/or modify acp_board configurations
54			*
55			* @event paul999.ajaxshoutbox.shoutbox_config_edit_add
56			* @var array display_vars Array of config values to display and process
57			* @var string mode Mode of the config page we are displaying
58			* @var boolean submit Do we display the form or process the submission
59			* @since 1.0.0-b2
60			*/
61			$vars = array('display_vars', 'mode', 'submit');
62			extract($phpbb_dispatcher->trigger_event('paul999.ajaxshoutbox.shoutbox_config_edit_add', compact($vars)));
63
64			$this->new_config = $config;
65			// Copied from acp_board.php
66			$cfg_array = (isset($_REQUEST['config'])) ? utf8_normalize_nfc($request->variable('config', array('' => ''), true)) : $this->new_config;
67			$error = array();
68
69			// We validate the complete config if wished
70			validate_config_vars($display_vars['vars'], $cfg_array, $error);
71
72			if ($submit && !check_form_key($form_key))
73			{
74			$error[] = $user->lang['FORM_INVALID'];
75			}
76			// Do not write values if there is an error
77			if (sizeof($error))
78			{
79			$submit = false;
80			}
81
82			// We go through the display_vars to make sure no one is trying to set variables he/she is not allowed to...
83			foreach ($display_vars['vars'] as $config_name => $null)
84			{
85			if (!isset($cfg_array[$config_name]) \|\| strpos($config_name, 'legend') !== false \|\| $config_name == 'ajaxshoutbox_validation_id' \|\| $config_name == 'ajaxshoutbox_push_disabled')
86			{
87			continue;
88			}
89
90			$this->new_config[$config_name] = $config_value = $cfg_array[$config_name];
91
92			if ($submit)
93			{
94			$config->set($config_name, $config_value);
95			}
96			}
97
98			if ($submit)
99			{
100			$phpbb_log->add('admin', $user->data['user_id'], $user->ip, 'LOG_AJAX_SHOUTBOX_CONFIG_' . strtoupper($mode));
101
102			$message = $user->lang('CONFIG_UPDATED');
103			$message_type = E_USER_NOTICE;
104
105			trigger_error($message . adm_back_link($this->u_action), $message_type);
106			}
107
108			$this->tpl_name = 'acp_board';
109			$this->page_title = $display_vars['title'];
110
111			$template->assign_vars(array(
112			'L_TITLE' => $user->lang[$display_vars['title']],
113			'L_TITLE_EXPLAIN' => $user->lang[$display_vars['title'] . '_EXPLAIN'],
114
115			'S_ERROR' => (sizeof($error)) ? true : false,
116			'ERROR_MSG' => implode('<br />', $error),
117
118			'U_ACTION' => $this->u_action,
119			));
120
121			// Output relevant page
122			foreach ($display_vars['vars'] as $config_key => $vars)
123			{
124			if (!is_array($vars) && strpos($config_key, 'legend') === false)
125			{
126			continue;
127			}
128
129			if (strpos($config_key, 'legend') !== false)
130			{
131			$template->assign_block_vars('options', array(
132			'S_LEGEND' => true,
133			'LEGEND' => (isset($user->lang[$vars])) ? $user->lang[$vars] : $vars)
134			);
135
136			continue;
137			}
138
139			$type = explode(':', $vars['type']);
140
141			$l_explain = '';
142			if ($vars['explain'] && isset($user->lang[$vars['lang'] . '_EXPLAIN']))
143			{
144			$l_explain = $user->lang[$vars['lang'] . '_EXPLAIN'];
145			}
146
147			$content = build_cfg_template($type, $config_key, $this->new_config, $config_key, $vars);
148
149			if (empty($content))
150			{
151			continue;
152			}
153
154			$template->assign_block_vars('options', array(
155			'KEY' => $config_key,
156			'TITLE' => (isset($user->lang[$vars['lang']])) ? $user->lang[$vars['lang']] : $vars['lang'],
157			'S_EXPLAIN' => $vars['explain'],
158			'TITLE_EXPLAIN' => $l_explain,
159			'CONTENT' => $content,
160
161			));
162
163			unset($display_vars['vars'][$config_key]);
164			}
165			}
166
167			/**
168			* Set the date format.
169			* @param $value
170			* @param $key
171			*
172			* @return string
173			*/
174			function dateformat_select($value, $key)
			0 ignored issues – show Best Practice introduced 2015-12-09 13:37 UTC by Report Bug Copy Issue Report It is generally recommended to explicitly declare the visibility for methods. Adding explicit visibility (`private`, `protected`, or `public`) is generally recommend to communicate to other developers how, and from where this method is intended to be used. Loading history...
175			{
176			global $user, $config;
			0 ignored issues – show Compatibility Best Practice introduced 2015-12-19 20:34 UTC by Report Bug Copy Issue Report Use of `global` functionality is not recommended; it makes your code harder to test, and less reusable. Instead of relying on `global` state, we recommend one of these alternatives: 1. Pass all data via parameters function myFunction($a, $b) { // Do something } 2. Create a class that maintains your state class MyClass { private $a; private $b; public function __construct($a, $b) { $this->a = $a; $this->b = $b; } public function myFunction() { // Do something } } Loading history...
177			// Let the format_date function operate with the acp values
178			$old_tz = $user->timezone;
179			try
180			{
181			$user->timezone = new \DateTimeZone($config['board_timezone']);
182			}
183			catch (\Exception $e)
184			{
185			// If the board timezone is invalid, we just use the users timezone.
186			}
187			$dateformat_options = '';
188			foreach ($user->lang['dateformats'] as $format => $null)
189			{
190			if (strpos($format, '\|') === 0) // Skip relative formats!
191			{
192			continue;
193			}
194
195			$dateformat_options .= '<option value="' . $format . '"' . (($format == $value) ? ' selected="selected"' : '') . '>';
196			$dateformat_options .= $user->format_date(time(), $format, false) . ((strpos($format, '\|') !== false) ? $user->lang['VARIANT_DATE_SEPARATOR'] . $user->format_date(time(), $format, true) : '');
197			$dateformat_options .= '</option>';
198			}
199			$dateformat_options .= '<option value="custom"';
200			if (!isset($user->lang['dateformats'][$value]))
201			{
202			$dateformat_options .= ' selected="selected"';
203			}
204			$dateformat_options .= '>' . $user->lang['CUSTOM_DATEFORMAT'] . '</option>';
205			// Reset users date options
206			$user->timezone = $old_tz;
207			return "<select name=\"dateoptions\" id=\"dateoptions\" onchange=\"if (this.value == 'custom') { document.getElementById('" . addslashes($key) . "').value = '" . addslashes($value) . "'; } else { document.getElementById('" . addslashes($key) . "').value = this.value; }\">$dateformat_options</select>
208			<input type=\"text\" name=\"config[$key]\" id=\"$key\" value=\"$value\" maxlength=\"30\" />";
209			}
210			}
211

paul999 / ajax-shoutbox-ext

Push — develop-stable ( 2a7b07...4a69f7 )

acp_module::main() F

Complexity

Size

Duplication

Importance

How to fix Long Method Complexity

Long Method

1. Pass all data via parameters

2. Create a class that maintains your state

1. Pass all data via parameters

2. Create a class that maintains your state

1. Pass all data via parameters

2. Create a class that maintains your state

Duplication Side-by-Side

Filter issues like