TemplateParser - Code Metrics - Inspection of "Refactoring" - evolution-cms/evolution - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Pull Request — develop (#716)

by Agel_Nash

created 2018-06-07 21:08 UTC

TemplateParser B

↳ Parent: Project

Complexity

Total Complexity

Size/Duplication

Total Lines	178
Duplicated Lines	8.99 %

Coupling/Cohesion

Components	0
Dependencies	2

Importance

Changes

Metric	Value
dl	16
loc	178
rs	8.3396
c	0
b	0
f	0
wmc	44
lcom	0
cbo	2

2 Methods

Rating	Name	Duplication	Size	Complexity
C	output()	0	51	14
F	render()	16	112	30

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

<?php namespace EvolutionCMS\Legacy;

Class TemplateParser
{

    /**
     * @param array $config [action, tabs, toArray]
     * @param array $data
     * @return string

     */
    public function output($config = array(), $data = array()) {
// 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);
    }
}
        $modx = evolutionCMS();

        $output = '';
        $action = !empty($config['action']) ? $config['action'] : (!empty($_REQUEST['a']) ? $_REQUEST['a'] : '');
        $tab = isset($config['tab']) ? ' AND tab IN(' . $config['tab'] . ')' : '';

        if($action) {
            $sql = $modx->db->query('SELECT t1.*, IF(t1.alias=\'\',t1.name,t1.alias) AS alias, t2.category AS category_name
			FROM ' . $modx->getFullTableName('system_templates') . ' AS t1
			INNER JOIN ' . $modx->getFullTableName('categories') . ' AS t2 ON t2.id=t1.category
			WHERE t1.action IN(' . $action . ') ' . $tab . '
			ORDER BY t1.tab ASC, t1.rank ASC');

            if($modx->db->getRecordCount($sql)) {
                $tabs = array();
                while($row = $modx->db->getRow($sql)) {
                    if(!$row['value'] && !empty($data[$row['name']])) {
                        $row['value'] = $data[$row['name']];
                    }
                    $tabs[$row['tab']]['category_name'] = $row['category_name'];
                    $tabs[$row['tab']][$row['name']] = $this->render($row);
                }

                if(!empty($config['toArray'])) {
                    $output = $tabs;
                } else {
                    $output .= '<div class="tab-pane" id="pane_' . $action . '">';
                    $output .= '
					<script type="text/javascript">
						var pane_' . $action . ' = new WebFXTabPane(document.getElementById("pane_' . $action . '"), ' . ($modx->config['remember_last_tab'] == 1 ? 'true' : 'false') . ');
					</script>';

                    foreach($tabs as $idTab => $tab) {
                        $output .= '<div class="tab-page" id="tab_' . $action . '_' . $idTab . '">';
                        $output .= '
						<h2 class="tab">' . (!empty($config['tabs'][$idTab]) ? $config['tabs'][$idTab] : $tab['category_name']) . '</h2>
						<script type="text/javascript">pane_' . $action . '.addTabPage(document.getElementById("tab_' . $action . '_' . $idTab . '"));</script>';
                        unset($tab['category_name']);
                        foreach($tab as $item) {
                            $output .= $item;
                        }
                        $output .= '</div>';
                    }
                    $output .= '</div>';
                }
            }
        }

        return $output;
    }

    /**
     * @param array $data
     * @return string
     */
    private function render($data) {
// 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);
    }
}
        $modx = evolutionCMS(); global $_lang, $_country_lang;
// Bad
$a = 5; $b = 6; $c = 7;

// Good
$a = 5;
$b = 6;
$c = 7;

        $data['lang.name'] = (isset($_lang[$data['alias']]) ? $_lang[$data['alias']] : $data['alias']);
        $data['value'] = (isset($_POST[$data['name']][$data['value']]) ? $_POST[$data['name']][$data['value']] : (isset($data['value']) ? $modx->htmlspecialchars($data['value']) : ''));
        $data['readonly'] = ($data['readonly'] ? ' readonly' : '');

        $output = '';
        $output .= '<div class="form-group row">';

        switch($data['type']) {

            case 'text':
                $output .= '<label class="col-sm-3" for="[+name+]">[+lang.name+]</label>
					<div class="col-sm-7">
					<input type="text" name="[+name+]" class="form-control" id="[+name+]" value="[+value+]" onChange="documentDirty=true;"[+readonly+] />';
                $output .= $data['content'];
                $output .= '</div>';

                break;

            case 'textarea':
                $output .= '<label class="col-sm-3" for="[+name+]">[+lang.name+]</label>
					<div class="col-sm-7">
					<textarea name="[+name+]" class="form-control" id="[+name+]" onChange="documentDirty=true;"[+readonly+]>[+value+]</textarea>';
                $output .= $data['content'];
                $output .= '</div>';

                break;

            case 'date':
                $data['value'] = (isset($_POST[$data['name']][$data['value']]) ? $modx->toDateFormat($_POST[$data['name']][$data['value']]) : (isset($data['value']) ? $modx->toDateFormat($data['value']) : ''));
                $output .= '<label class="col-sm-3" for="[+name+]">[+lang.name+]</label>
					<div class="col-sm-7">
					<input type="text" name="[+name+]" class="form-control DatePicker" id="[+name+]" value="[+value+]" onChange="documentDirty=true;"[+readonly+] />';
                $output .= $data['content'];
                $output .= '</div>';

                break;

            case 'select':
                $output .= '<label class="col-sm-3" for="[+name+]">[+lang.name+]</label>';
                $output .= '<div class="col-sm-7">';
                $output .= '<select name="[+name+]" class="form-control" id="[+name+]" onChange="documentDirty=true;">';
                if($data['name'] == 'country' && isset($_country_lang)) {
                    $chosenCountry = isset($_POST['country']) ? $_POST['country'] : $data['country'];
                    $output .= '<option value=""' . (!isset($chosenCountry) ? ' selected' : '') . '>&nbsp;</option>';
                    foreach($_country_lang as $key => $value) {
                        $output .= '<option value="' . $key . '"' . (isset($chosenCountry) && $chosenCountry == $key ? ' selected' : '') . '>' . $value . '</option>';
                    }
                } else {
                    if($data['elements']) {
                        $elements = explode('||', $data['elements']);
                        foreach($elements as $key => $value) {
                            $value = explode('==', $value);
                            $output .= '<option value="' . $value[1] . '">' . (isset($_lang[$value[0]]) ? $_lang[$value[0]] : $value[0]) . '</option>';
                        }
                    }
                }
                $output .= '</select>';
                $output .= $data['content'];
                $output .= '</div>';

                break;

            case 'checkbox':
                $output .= '<label class="col-sm-3" for="[+name+]">[+lang.name+]</label>';
                $output .= '<div class="col-sm-7">';
                $output .= '<input type="checkbox" name="[+name+]" class="form-control" id="[+name+]" value="[+value+]" onChange="documentDirty=true;"[+readonly+] />';
                if($data['elements']) {
                    $elements = explode('||', $data['elements']);
                    foreach($elements as $key => $value) {
                        $value = explode('==', $value);
                        $output .= '<br /><input type="checkbox" name="' . $value[0] . '" class="form-control" id="' . $value[0] . '" value="' . $value[1] . '" onChange="documentDirty=true;"[+readonly+] /> ' . (isset($_lang[$value[0]]) ? $_lang[$value[0]] : $value[0]);
                    }
                }
                $output .= $data['content'];
                $output .= '</div>';

                break;

            case 'radio':
                $output .= '<label class="col-sm-3" for="[+name+]">[+lang.name+]</label>';
                $output .= '<div class="col-sm-7">';
                $output .= '<input type="radio" name="[+name+]" class="form-control" id="[+name+]" value="[+value+]" onChange="documentDirty=true;"[+readonly+] />';
                if($data['elements']) {
                    $elements = explode('||', $data['elements']);
                    foreach($elements as $key => $value) {
                        $value = explode('==', $value);
                        $output .= '<br /><input type="radio" name="[+name+]" class="form-control" id="[+name+]_' . $key . '" value="' . $value[1] . '" onChange="documentDirty=true;"[+readonly+] /> ' . (isset($_lang[$value[0]]) ? $_lang[$value[0]] : $value[0]);
                    }
                }
                $output .= $data['content'];
                $output .= '</div>';

                break;

            case 'custom':
                $output .= '<label class="col-sm-3" for="[+name+]">[+lang.name+]</label>';
                $output .= '<div class="col-sm-7">';
                $output .= $data['content'];
                $output .= '</div>';

                break;
        }

        $output .= '</div>';

        $output = $modx->parseText($output, $data);

        return $output;
    }

}


1		<?php namespace EvolutionCMS\Legacy;
2
3		Class TemplateParser
4		{
5
6		/**
7		* @param array $config [action, tabs, toArray]
8		* @param array $data
9		* @return string
		0 ignored issues – show Documentation introduced 2018-02-21 15:53 UTC by Report Bug Copy Issue Report Should the return type not be `array\|string`? Also, consider making the array more specific, something like `array<String>`, or `String[]`. This check compares the return type specified in the `@return` annotation of a function or method doc comment with the types returned by the function and raises an issue if they mismatch. If the return type contains the type array, this check recommends the use of a more specific type like `String[]` or `array<String>`. Loading history...
10		*/
11		public function output($config = array(), $data = array()) {
		0 ignored issues – show Coding Style introduced 2018-02-21 15:53 UTC by Report Bug Copy Issue Report `output` 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...
12		$modx = evolutionCMS();
13
14		$output = '';
15		$action = !empty($config['action']) ? $config['action'] : (!empty($_REQUEST['a']) ? $_REQUEST['a'] : '');
16		$tab = isset($config['tab']) ? ' AND tab IN(' . $config['tab'] . ')' : '';
17
18		if($action) {
19		$sql = $modx->db->query('SELECT t1.*, IF(t1.alias=\'\',t1.name,t1.alias) AS alias, t2.category AS category_name
20		FROM ' . $modx->getFullTableName('system_templates') . ' AS t1
21		INNER JOIN ' . $modx->getFullTableName('categories') . ' AS t2 ON t2.id=t1.category
22		WHERE t1.action IN(' . $action . ') ' . $tab . '
23		ORDER BY t1.tab ASC, t1.rank ASC');
24
25		if($modx->db->getRecordCount($sql)) {
26		$tabs = array();
27		while($row = $modx->db->getRow($sql)) {
28		if(!$row['value'] && !empty($data[$row['name']])) {
29		$row['value'] = $data[$row['name']];
30		}
31		$tabs[$row['tab']]['category_name'] = $row['category_name'];
32		$tabs[$row['tab']][$row['name']] = $this->render($row);
33		}
34
35		if(!empty($config['toArray'])) {
36		$output = $tabs;
37		} else {
38		$output .= '<div class="tab-pane" id="pane_' . $action . '">';
39		$output .= '
40		<script type="text/javascript">
41		var pane_' . $action . ' = new WebFXTabPane(document.getElementById("pane_' . $action . '"), ' . ($modx->config['remember_last_tab'] == 1 ? 'true' : 'false') . ');
42		</script>';
43
44		foreach($tabs as $idTab => $tab) {
45		$output .= '<div class="tab-page" id="tab_' . $action . '_' . $idTab . '">';
46		$output .= '
47		<h2 class="tab">' . (!empty($config['tabs'][$idTab]) ? $config['tabs'][$idTab] : $tab['category_name']) . '</h2>
48		<script type="text/javascript">pane_' . $action . '.addTabPage(document.getElementById("tab_' . $action . '_' . $idTab . '"));</script>';
49		unset($tab['category_name']);
50		foreach($tab as $item) {
51		$output .= $item;
52		}
53		$output .= '</div>';
54		}
55		$output .= '</div>';
56		}
57		}
58		}
59
60		return $output;
61		}
62
63		/**
64		* @param array $data
65		* @return string
66		*/
67		private function render($data) {
		0 ignored issues – show Coding Style introduced 2018-02-26 22:56 UTC by Report Bug Copy Issue Report `render` uses the super-global variable `$_POST` 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...
68		$modx = evolutionCMS(); global $_lang, $_country_lang;
		0 ignored issues – show Coding Style introduced 2018-02-26 22:56 UTC by Report Bug Copy Issue Report It is generally recommended to place each PHP statement on a line by itself. Let’s take a look at an example: // Bad $a = 5; $b = 6; $c = 7; // Good $a = 5; $b = 6; $c = 7; Loading history...
69
70		$data['lang.name'] = (isset($_lang[$data['alias']]) ? $_lang[$data['alias']] : $data['alias']);
71		$data['value'] = (isset($_POST[$data['name']][$data['value']]) ? $_POST[$data['name']][$data['value']] : (isset($data['value']) ? $modx->htmlspecialchars($data['value']) : ''));
72		$data['readonly'] = ($data['readonly'] ? ' readonly' : '');
73
74		$output = '';
75		$output .= '<div class="form-group row">';
76
77		switch($data['type']) {
78
79		case 'text':
80		$output .= '<label class="col-sm-3" for="[+name+]">[+lang.name+]</label>
81		<div class="col-sm-7">
82		<input type="text" name="[+name+]" class="form-control" id="[+name+]" value="[+value+]" onChange="documentDirty=true;"[+readonly+] />';
83		$output .= $data['content'];
84		$output .= '</div>';
85
86		break;
87
88		case 'textarea':
89		$output .= '<label class="col-sm-3" for="[+name+]">[+lang.name+]</label>
90		<div class="col-sm-7">
91		<textarea name="[+name+]" class="form-control" id="[+name+]" onChange="documentDirty=true;"[+readonly+]>[+value+]</textarea>';
92		$output .= $data['content'];
93		$output .= '</div>';
94
95		break;
96
97		case 'date':
98		$data['value'] = (isset($_POST[$data['name']][$data['value']]) ? $modx->toDateFormat($_POST[$data['name']][$data['value']]) : (isset($data['value']) ? $modx->toDateFormat($data['value']) : ''));
99		$output .= '<label class="col-sm-3" for="[+name+]">[+lang.name+]</label>
100		<div class="col-sm-7">
101		<input type="text" name="[+name+]" class="form-control DatePicker" id="[+name+]" value="[+value+]" onChange="documentDirty=true;"[+readonly+] />';
102		$output .= $data['content'];
103		$output .= '</div>';
104
105		break;
106
107		case 'select':
108		$output .= '<label class="col-sm-3" for="[+name+]">[+lang.name+]</label>';
109		$output .= '<div class="col-sm-7">';
110		$output .= '<select name="[+name+]" class="form-control" id="[+name+]" onChange="documentDirty=true;">';
111		if($data['name'] == 'country' && isset($_country_lang)) {
112		$chosenCountry = isset($_POST['country']) ? $_POST['country'] : $data['country'];
113		$output .= '<option value=""' . (!isset($chosenCountry) ? ' selected' : '') . '> </option>';
114		foreach($_country_lang as $key => $value) {
115		$output .= '<option value="' . $key . '"' . (isset($chosenCountry) && $chosenCountry == $key ? ' selected' : '') . '>' . $value . '</option>';
116		}
117	View Code Duplication	} else {
118		if($data['elements']) {
119		$elements = explode('\|\|', $data['elements']);
120		foreach($elements as $key => $value) {
121		$value = explode('==', $value);
122		$output .= '<option value="' . $value[1] . '">' . (isset($_lang[$value[0]]) ? $_lang[$value[0]] : $value[0]) . '</option>';
123		}
124		}
125		}
126		$output .= '</select>';
127		$output .= $data['content'];
128		$output .= '</div>';
129
130		break;
131
132		case 'checkbox':
133		$output .= '<label class="col-sm-3" for="[+name+]">[+lang.name+]</label>';
134		$output .= '<div class="col-sm-7">';
135		$output .= '<input type="checkbox" name="[+name+]" class="form-control" id="[+name+]" value="[+value+]" onChange="documentDirty=true;"[+readonly+] />';
136		if($data['elements']) {
137		$elements = explode('\|\|', $data['elements']);
138		foreach($elements as $key => $value) {
139		$value = explode('==', $value);
140		$output .= '<br /><input type="checkbox" name="' . $value[0] . '" class="form-control" id="' . $value[0] . '" value="' . $value[1] . '" onChange="documentDirty=true;"[+readonly+] /> ' . (isset($_lang[$value[0]]) ? $_lang[$value[0]] : $value[0]);
141		}
142		}
143		$output .= $data['content'];
144		$output .= '</div>';
145
146		break;
147
148		case 'radio':
149		$output .= '<label class="col-sm-3" for="[+name+]">[+lang.name+]</label>';
150		$output .= '<div class="col-sm-7">';
151		$output .= '<input type="radio" name="[+name+]" class="form-control" id="[+name+]" value="[+value+]" onChange="documentDirty=true;"[+readonly+] />';
152	View Code Duplication	if($data['elements']) {
153		$elements = explode('\|\|', $data['elements']);
154		foreach($elements as $key => $value) {
155		$value = explode('==', $value);
156		$output .= '<br /><input type="radio" name="[+name+]" class="form-control" id="[+name+]_' . $key . '" value="' . $value[1] . '" onChange="documentDirty=true;"[+readonly+] /> ' . (isset($_lang[$value[0]]) ? $_lang[$value[0]] : $value[0]);
157		}
158		}
159		$output .= $data['content'];
160		$output .= '</div>';
161
162		break;
163
164		case 'custom':
165		$output .= '<label class="col-sm-3" for="[+name+]">[+lang.name+]</label>';
166		$output .= '<div class="col-sm-7">';
167		$output .= $data['content'];
168		$output .= '</div>';
169
170		break;
171		}
172
173		$output .= '</div>';
174
175		$output = $modx->parseText($output, $data);
176
177		return $output;
178		}
179
180		}
181

evolution-cms / evolution