BlockManager - Code Metrics - Inspection of "initial commit" - sheadawson/silverstripe-blocks - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Pull Request — master (#111)

unknown

created 2017-01-13 10:57 UTC

BlockManager B

↳ Parent: Project

Complexity

Total Complexity

Size/Duplication

Total Lines	229
Duplicated Lines	8.73 %

Coupling/Cohesion

Components	1
Dependencies	0

Importance

Changes

Metric	Value
wmc	50
lcom	1
cbo	0
dl	20
loc	229
rs	8.6206
c	0
b	0
f	0

12 Methods

Rating	Name	Duplication	Size	Complexity
A	__construct()	0	4	1
B	getAreasForTheme()	0	20	8
C	getAreasForPageType()	20	46	12
D	getBlockClasses()	0	28	9
A	getTheme()	0	12	4
A	getThemeConfig()	0	7	3
A	getUseBlockSets()	0	6	2
A	getExcludeFromPageTypes()	0	6	2
A	getWhiteListedPageTypes()	0	5	2
A	getBlackListedPageTypes()	0	7	3
A	getUseExtraCSSClasses()	0	6	2
A	getPrefixDefaultCSSClasses()	0	6	2

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

<?php

namespace SheaDawson\Blocks;

use SilverStripe\Core\Object;
use SilverStripe\ORM\ArrayLib;
use SilverStripe\SiteConfig\SiteConfig;
use SilverStripe\Core\Config\Config;
use SilverStripe\Core\ClassInfo;
use SilverStripe\Forms\FormField;
/**
 * BlockManager.
 *
 * @author Shea Dawson <[email protected]>
 */
class BlockManager extends Object
{
	/**
	 * Define areas and config on a per theme basis.
	 *
	 * @var array
	 **/
	private static $themes = array();


	/**
	 * Use default ContentBlock class.
	 *
	 * @var bool
	 **/
	private static $use_default_blocks = true;


	/**
	 * Show a block area preview button in CMS
	 *
	 * @var bool
	 **/
	private static $block_area_preview = true;


	public function __construct()
	{
		parent::__construct();
	}

	/**
	 * Gets an array of all areas defined for the current theme.
	 *
	 * @param string $theme
	 * @param bool   $keyAsValue
	 *
	 * @return array $areas
	 **/
	public function getAreasForTheme($theme = null, $keyAsValue = true)
	{
		$theme = $theme ? $theme : $this->getTheme();
		if (!$theme) {
			return false;
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());
}
		}
		$config = $this->config()->get('themes');
		if (!isset($config[$theme]['areas'])) {
			return false;
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());
}
		}
		$areas = $config[$theme]['areas'];
		$areas = $keyAsValue ? ArrayLib::valuekey(array_keys($areas)) : $areas;
		if (count($areas)) {
			foreach ($areas as $k => $v) {
				$areas[$k] = $keyAsValue ? FormField::name_to_label($k) : $v;
			}
		}

		return $areas;
	}

	/**
	 * Gets an array of all areas defined for the current theme that are compatible
	 * with pages of type $class.
	 *
	 * @param string $class
	 *
	 * @return array $areas
	 **/
	public function getAreasForPageType($class)
	{
		$areas = $this->getAreasForTheme(null, false);

		if (!$areas) {

			return false;
		}

		foreach ($areas as $area => $config) {
			if (!is_array($config)) {
				continue;
			}

			if (isset($config['except'])) {

				$except = $config['except'];
				if (is_array($except)
					? in_array($class, $except)
					: $except == $class
				) {
					unset($areas[$area]);
					continue;
				}
			}

			if (isset($config['only'])) {

				$only = $config['only'];
				if (is_array($only)
					? !in_array($class, $only)
					: $only != $class
				) {
					unset($areas[$area]);
					continue;
				}
			}
		}

		if (count($areas)) {
			foreach ($areas as $k => $v) {
				$areas[$k] = _t('Block.BlockAreaName.'.$k, FormField::name_to_label($k));
			}

			return $areas;
		} else {
			return $areas;
		}
	}

	public function getBlockClasses()
	{
		$classes = ArrayLib::valuekey(ClassInfo::subclassesFor("SheaDawson\Blocks\model\Block"));
		array_shift($classes);
		foreach ($classes as $k => $v) {
			$classes[$k] = singleton($k)->singular_name();
		}

		$themeConfig = $this->getThemeConfig();

		if (isset($themeConfig['use_default_blocks']) && !$themeConfig['use_default_blocks']) {
	        unset($classes['ContentBlock']);
	    } else if (!$this->config()->use_default_blocks) {
	        unset($classes['ContentBlock']);
	    }

		$disabledArr = Config::inst()->get("BlockManager", 'disabled_blocks') ? Config::inst()->get("BlockManager", 'disabled_blocks') : array();
		if (isset($themeConfig['disabled_blocks'])) {
		    $disabledArr = array_merge($disabledArr, $themeConfig['disabled_blocks']);
		}
		if (count($disabledArr)) {
			foreach ($disabledArr as $k => $v) {
				unset($classes[$v]);
			}
		}

		return $classes;
	}

	/*
	 * Get the current/active theme or 'default' to support theme-less sites
	 */
	private function getTheme()
	{
		$currentTheme = Config::inst()->get('SSViewer', 'theme');

		// check directly on SiteConfig incase ContentController hasn't set
		// the theme yet in ContentController->init()
		if (!$currentTheme && class_exists('SiteConfig')) {
			$currentTheme = SiteConfig::current_site_config()->Theme;
		}

		return $currentTheme ? $currentTheme : 'default';
	}

	/*
	 * Get the block config for the current theme
	 */
	private function getThemeConfig()
	{
		$theme = $this->getTheme();
		$config = $this->config()->get('themes');

		return $theme && isset($config[$theme]) ? $config[$theme] : null;
	}

	/*
	 * Usage of BlockSets configurable from yaml
	 */
	public function getUseBlockSets()
	{
		$config = $this->getThemeConfig();

		return isset($config['use_blocksets']) ? $config['use_blocksets'] : true;
	}

	/*
	 * Exclusion of blocks from page types defined in yaml
	 */
	public function getExcludeFromPageTypes()
	{
		$config = $this->getThemeConfig();

		return isset($config['exclude_from_page_types']) ? $config['exclude_from_page_types'] : array();
	}

	/*
	 * getWhiteListedPageTypes optionally configured by the developer
	 */
	public function getWhiteListedPageTypes()
	{
		$config = $this->getThemeConfig();
		return isset($config['pagetype_whitelist']) ? $config['pagetype_whitelist'] : array();
	}

	/*
	 * getBlackListedPageTypes optionally configured by the developer
	 * Includes blacklisted page types defined in the old exclude_from_page_types array
	 */
	public function getBlackListedPageTypes()
	{
		$config = $this->getThemeConfig();
		$legacy = isset($config['exclude_from_page_types']) ? $config['exclude_from_page_types'] : array();
		$current = isset($config['pagetype_blacklist']) ? $config['pagetype_blacklist'] : array();
		return array_merge($legacy, $current);
	}

	/*
	 * Usage of extra css classes configurable from yaml
	 */
	public function getUseExtraCSSClasses()
	{
		$config = $this->getThemeConfig();

		return isset($config['use_extra_css_classes']) ? $config['use_extra_css_classes'] : false;
	}

	/*
	 * Prefix for the default CSSClasses
	 */
	public function getPrefixDefaultCSSClasses()
	{
		$config = $this->getThemeConfig();

		return isset($config['prefix_default_css_classes']) ? $config['prefix_default_css_classes'] : false;
	}
}


1		<?php
2
3		namespace SheaDawson\Blocks;
4
5		use SilverStripe\Core\Object;
6		use SilverStripe\ORM\ArrayLib;
7		use SilverStripe\SiteConfig\SiteConfig;
8		use SilverStripe\Core\Config\Config;
9		use SilverStripe\Core\ClassInfo;
10		use SilverStripe\Forms\FormField;
11		/**
12		* BlockManager.
13		*
14		* @author Shea Dawson <[email protected]>
15		*/
16		class BlockManager extends Object
17		{
18		/**
19		* Define areas and config on a per theme basis.
20		*
21		* @var array
22		**/
23		private static $themes = array();
		0 ignored issues – show Unused Code introduced 2016-05-23 01:41 UTC by Report Bug Copy Issue Report The property `$themes` is not used and could be removed. This check marks private properties in classes that are never used. Those properties can be removed. Loading history...
24
25		/**
26		* Use default ContentBlock class.
27		*
28		* @var bool
29		**/
30		private static $use_default_blocks = true;
		0 ignored issues – show Unused Code introduced 2016-05-23 01:41 UTC by Report Bug Copy Issue Report The property `$use_default_blocks` is not used and could be removed. This check marks private properties in classes that are never used. Those properties can be removed. Loading history...
31
32		/**
33		* Show a block area preview button in CMS
34		*
35		* @var bool
36		**/
37		private static $block_area_preview = true;
		0 ignored issues – show Unused Code introduced 2016-05-23 01:41 UTC by Report Bug Copy Issue Report The property `$block_area_preview` is not used and could be removed. This check marks private properties in classes that are never used. Those properties can be removed. Loading history...
38
39		public function __construct()
40		{
41		parent::__construct();
42		}
43
44		/**
45		* Gets an array of all areas defined for the current theme.
46		*
47		* @param string $theme
48		* @param bool $keyAsValue
49		*
50		* @return array $areas
51		**/
52		public function getAreasForTheme($theme = null, $keyAsValue = true)
53		{
54		$theme = $theme ? $theme : $this->getTheme();
55		if (!$theme) {
56		return false;
		0 ignored issues – show Bug Best Practice introduced 2017-01-13 10:32 UTC by Report Bug Copy Issue Report The return type of `return false;` (`false`) is incompatible with the return type documented by `SheaDawson\Blocks\BlockManager::getAreasForTheme` of type `array`. 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...
57		}
58		$config = $this->config()->get('themes');
59		if (!isset($config[$theme]['areas'])) {
60		return false;
		0 ignored issues – show Bug Best Practice introduced 2017-01-13 10:32 UTC by Report Bug Copy Issue Report The return type of `return false;` (`false`) is incompatible with the return type documented by `SheaDawson\Blocks\BlockManager::getAreasForTheme` of type `array`. 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...
61		}
62		$areas = $config[$theme]['areas'];
63		$areas = $keyAsValue ? ArrayLib::valuekey(array_keys($areas)) : $areas;
64		if (count($areas)) {
65		foreach ($areas as $k => $v) {
66		$areas[$k] = $keyAsValue ? FormField::name_to_label($k) : $v;
67		}
68		}
69
70		return $areas;
71		}
72
73		/**
74		* Gets an array of all areas defined for the current theme that are compatible
75		* with pages of type $class.
76		*
77		* @param string $class
78		*
79		* @return array $areas
80		**/
81		public function getAreasForPageType($class)
82		{
83		$areas = $this->getAreasForTheme(null, false);
84
85		if (!$areas) {
		0 ignored issues – show Bug Best Practice introduced 2016-05-23 01:41 UTC by Report Bug Copy Issue Report The expression `$areas` of type `array` is implicitly converted to a boolean; are you sure this is intended? If so, consider using `empty($expr)` instead to make it clear that you intend to check for an array without elements. This check marks implicit conversions of arrays to boolean values in a comparison. While in PHP an empty array is considered to be equal (but not identical) to false, this is not always apparent. Consider making the comparison explicit by using `empty(..)` or `! empty(...)` instead. Loading history...
86		return false;
87		}
88
89		foreach ($areas as $area => $config) {
90		if (!is_array($config)) {
91		continue;
92		}
93
94	View Code Duplication	if (isset($config['except'])) {
		0 ignored issues – show Duplication introduced 2016-05-23 01:41 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...
95		$except = $config['except'];
96		if (is_array($except)
97		? in_array($class, $except)
98		: $except == $class
99		) {
100		unset($areas[$area]);
101		continue;
102		}
103		}
104
105	View Code Duplication	if (isset($config['only'])) {
		0 ignored issues – show Duplication introduced 2016-05-23 01:41 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...
106		$only = $config['only'];
107		if (is_array($only)
108		? !in_array($class, $only)
109		: $only != $class
110		) {
111		unset($areas[$area]);
112		continue;
113		}
114		}
115		}
116
117		if (count($areas)) {
118		foreach ($areas as $k => $v) {
119		$areas[$k] = _t('Block.BlockAreaName.'.$k, FormField::name_to_label($k));
120		}
121
122		return $areas;
123		} else {
124		return $areas;
125		}
126		}
127
128		public function getBlockClasses()
129		{
130		$classes = ArrayLib::valuekey(ClassInfo::subclassesFor("SheaDawson\Blocks\model\Block"));
131		array_shift($classes);
132		foreach ($classes as $k => $v) {
133		$classes[$k] = singleton($k)->singular_name();
134		}
135
136		$themeConfig = $this->getThemeConfig();
137
138		if (isset($themeConfig['use_default_blocks']) && !$themeConfig['use_default_blocks']) {
139		unset($classes['ContentBlock']);
140		} else if (!$this->config()->use_default_blocks) {
141		unset($classes['ContentBlock']);
142		}
143
144		$disabledArr = Config::inst()->get("BlockManager", 'disabled_blocks') ? Config::inst()->get("BlockManager", 'disabled_blocks') : array();
145		if (isset($themeConfig['disabled_blocks'])) {
146		$disabledArr = array_merge($disabledArr, $themeConfig['disabled_blocks']);
147		}
148		if (count($disabledArr)) {
149		foreach ($disabledArr as $k => $v) {
150		unset($classes[$v]);
151		}
152		}
153
154		return $classes;
155		}
156
157		/*
158		* Get the current/active theme or 'default' to support theme-less sites
159		*/
160		private function getTheme()
161		{
162		$currentTheme = Config::inst()->get('SSViewer', 'theme');
163
164		// check directly on SiteConfig incase ContentController hasn't set
165		// the theme yet in ContentController->init()
166		if (!$currentTheme && class_exists('SiteConfig')) {
167		$currentTheme = SiteConfig::current_site_config()->Theme;
168		}
169
170		return $currentTheme ? $currentTheme : 'default';
171		}
172
173		/*
174		* Get the block config for the current theme
175		*/
176		private function getThemeConfig()
177		{
178		$theme = $this->getTheme();
179		$config = $this->config()->get('themes');
180
181		return $theme && isset($config[$theme]) ? $config[$theme] : null;
182		}
183
184		/*
185		* Usage of BlockSets configurable from yaml
186		*/
187		public function getUseBlockSets()
188		{
189		$config = $this->getThemeConfig();
190
191		return isset($config['use_blocksets']) ? $config['use_blocksets'] : true;
192		}
193
194		/*
195		* Exclusion of blocks from page types defined in yaml
196		*/
197		public function getExcludeFromPageTypes()
198		{
199		$config = $this->getThemeConfig();
200
201		return isset($config['exclude_from_page_types']) ? $config['exclude_from_page_types'] : array();
202		}
203
204		/*
205		* getWhiteListedPageTypes optionally configured by the developer
206		*/
207		public function getWhiteListedPageTypes()
208		{
209		$config = $this->getThemeConfig();
210		return isset($config['pagetype_whitelist']) ? $config['pagetype_whitelist'] : array();
211		}
212
213		/*
214		* getBlackListedPageTypes optionally configured by the developer
215		* Includes blacklisted page types defined in the old exclude_from_page_types array
216		*/
217		public function getBlackListedPageTypes()
218		{
219		$config = $this->getThemeConfig();
220		$legacy = isset($config['exclude_from_page_types']) ? $config['exclude_from_page_types'] : array();
221		$current = isset($config['pagetype_blacklist']) ? $config['pagetype_blacklist'] : array();
222		return array_merge($legacy, $current);
223		}
224
225		/*
226		* Usage of extra css classes configurable from yaml
227		*/
228		public function getUseExtraCSSClasses()
229		{
230		$config = $this->getThemeConfig();
231
232		return isset($config['use_extra_css_classes']) ? $config['use_extra_css_classes'] : false;
233		}
234
235		/*
236		* Prefix for the default CSSClasses
237		*/
238		public function getPrefixDefaultCSSClasses()
239		{
240		$config = $this->getThemeConfig();
241
242		return isset($config['prefix_default_css_classes']) ? $config['prefix_default_css_classes'] : false;
243		}
244		}
245

sheadawson / silverstripe-blocks

GitHub Access Token became invalid