CheckboxSetField - Code Metrics - Inspection of "Add new committers" - sminnee/silverstripe-framework - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — new-committers ( 29cb6f...bcba16 )

by Sam

created 2016-04-19 10:25 UTC

CheckboxSetField C

↳ Parent: Project

Complexity

Total Complexity

Size/Duplication

Total Lines	327
Duplicated Lines	18.65 %

Coupling/Cohesion

Components	1
Dependencies	9

Metric	Value
wmc	78
lcom	1
cbo	9
dl	61
loc	327
rs	5.4563

12 Methods

Rating	Name	Duplication	Size	Complexity
A	setDefaultItems()	0	4	1
A	getDefaultItems()	0	3	1
A	Type()	0	3	1
A	ExtraOptions()	0	3	1
A	Field()	0	11	1
F	getOptions()	21	101	28
B	setValue()	4	12	5
C	saveInto()	8	21	15
B	dataValue()	14	14	5
A	performDisabledTransformation()	0	6	1
C	performReadonlyTransformation()	0	48	14
B	validate()	14	35	5

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

<?php
/**
 * Displays a set of checkboxes as a logical group.
 *
 * ASSUMPTION -> IF you pass your source as an array, you pass values as an array too. Likewise objects are handled
 * the same.
 *
 * Example:
 * <code>
 * new CheckboxSetField(
 *  $name = "topics",
 *  $title = "I am interested in the following topics",
 *  $source = array(
 *      "1" => "Technology",
 *      "2" => "Gardening",
 *      "3" => "Cooking",
 *      "4" => "Sports"
 *  ),
 *  $value = "1"
 * );
 * </code>
 *
 * <b>Saving</b>
 * The checkbox set field will save its data in one of ways:
 * - If the field name matches a many-many join on the object being edited, that many-many join will be updated to
 *   link to the objects selected on the checkboxes.  In this case, the keys of your value map should be the IDs of
 *   the database records.
 * - If the field name matches a database field, a comma-separated list of values will be saved to that field.  The
 *   keys can be text or numbers.
 *
 * @todo Document the different source data that can be used
 * with this form field - e.g ComponentSet, ArrayList,
 * array. Is it also appropriate to accept so many different
 * types of data when just using an array would be appropriate?
 *
 * @package forms
 * @subpackage fields-basic
 */
class CheckboxSetField extends OptionsetField {

	/**
	 * @var array
	 */
	protected $defaultItems = array();

	/**
	 * @todo Explain different source data that can be used with this field,
	 * e.g. SQLMap, ArrayList or an array.
	 */
	public function Field($properties = array()) {
		Requirements::css(FRAMEWORK_DIR . '/css/CheckboxSetField.css');

		$properties = array_merge($properties, array(
			'Options' => $this->getOptions()
		));

		return $this->customise($properties)->renderWith(
			$this->getTemplates()
		);
	}

	/**
	 * @return ArrayList
	 */
	public function getOptions() {
		$odd = 0;

		$source = $this->source;
		$values = $this->value;
		$items = array();

		// Get values from the join, if available
		if(is_object($this->form)) {
			$record = $this->form->getRecord();
			if(!$values && $record && $record->hasMethod($this->name)) {
				$funcName = $this->name;
				$join = $record->$funcName();
				if($join) {
					foreach($join as $joinItem) {
						$values[] = $joinItem->ID;
					}
				}
			}
		}

		// Source is not an array
		if(!is_array($source) && !is_a($source, 'SQLMap')) {
			if(is_array($values)) {
				$items = $values;
			} else {
				// Source and values are DataObject sets.
				if($values && is_a($values, 'SS_List')) {
					foreach($values as $object) {
$collection = json_decode($data, true);
if ( ! is_array($collection)) {
    throw new \RuntimeException('$collection must be an array.');
}

foreach ($collection as $item) { /** ... */ }
						if(is_a($object, 'DataObject')) {
							$items[] = $object->ID;
						}
					}
				} elseif($values && is_string($values)) {

					if(!empty($values)) {
						$items = explode(',', $values);
						$items = str_replace('{comma}', ',', $items);
					} else {
						$items = array();
					}
				}
			}
		} else {
			// Sometimes we pass a singluar default value thats ! an array && !SS_List
			if($values instanceof SS_List || is_array($values)) {
				$items = $values;
			} else {

				if($values === null) {
					$items = array();
				}
				else {
					if(!empty($values)) {
						$items = explode(',', $values);
						$items = str_replace('{comma}', ',', $items);
					} else {
						$items = array();
					}
				}
			}
		}

		if(is_array($source)) {
			unset($source['']);
		}

		$options = array();

		if ($source == null) {
			$source = array();
		}

		foreach($source as $value => $item) {
			if($item instanceof DataObject) {
				$value = $item->ID;
				$title = $item->Title;
<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}
			} else {
				$title = $item;
			}

			$itemID = $this->ID() . '_' . preg_replace('/[^a-zA-Z0-9]/', '', $value);
			$odd = ($odd + 1) % 2;
			$extraClass = $odd ? 'odd' : 'even';
			$extraClass .= ' val' . preg_replace('/[^a-zA-Z0-9\-\_]/', '_', $value);

			$options[] = new ArrayData(array(
				'ID' => $itemID,
				'Class' => $extraClass,
				'Name' => "{$this->name}[{$value}]",
				'Value' => $value,
				'Title' => $title,
				'isChecked' => in_array($value, $items) || in_array($value, $this->defaultItems),
				'isDisabled' => $this->disabled || in_array($value, $this->disabledItems)
			));
		}

		$options = new ArrayList($options);

		$this->extend('updateGetOptions', $options);

		return $options;
	}

	/**
	 * Default selections, regardless of the {@link setValue()} settings.
	 * Note: Items marked as disabled through {@link setDisabledItems()} can still be
	 * selected by default through this method.
	 *
	 * @param Array $items Collection of array keys, as defined in the $source array
	 */
	public function setDefaultItems($items) {
		$this->defaultItems = $items;
		return $this;
	}

	/**
	 * @return Array
	 */
	public function getDefaultItems() {
		return $this->defaultItems;
	}

	/**
	 * Load a value into this CheckboxSetField
	 */
	public function setValue($value, $obj = null) {
		// If we're not passed a value directly, we can look for it in a relation method on the object passed as a
		// second arg
		if(!$value && $obj && $obj instanceof DataObject && $obj->hasMethod($this->name)) {

			$funcName = $this->name;
			$value = $obj->$funcName()->getIDList();
		}

		parent::setValue($value, $obj);


		return $this;
	}

	/**
	 * Save the current value of this CheckboxSetField into a DataObject.
	 * If the field it is saving to is a has_many or many_many relationship,
	 * it is saved by setByIDList(), otherwise it creates a comma separated
	 * list for a standard DB text/varchar field.
	 *
	 * @param DataObject $record The record to save into
	 */
	public function saveInto(DataObjectInterface $record) {
		$fieldname = $this->name;
		$relation = ($fieldname && $record && $record->hasMethod($fieldname)) ? $record->$fieldname() : null;
		if($fieldname && $record && $relation &&
			($relation instanceof RelationList || $relation instanceof UnsavedRelationList)) {
			$idList = array();
			if($this->value) foreach($this->value as $id => $bool) {
				if($bool) {
					$idList[] = $id;
				}
			}
			$relation->setByIDList($idList);
		} elseif($fieldname && $record) {

			if($this->value) {
				$this->value = str_replace(',', '{comma}', $this->value);
				$record->$fieldname = implode(',', (array) $this->value);
			} else {
				$record->$fieldname = '';
			}
		}
	}

	/**
	 * Return the CheckboxSetField value as a string
	 * selected item keys.
	 *
	 * @return string
	 */
	public function dataValue() {

		if($this->value && is_array($this->value)) {
			$filtered = array();
			foreach($this->value as $item) {
				if($item) {
					$filtered[] = str_replace(",", "{comma}", $item);
				}
			}

			return implode(',', $filtered);
		}

		return '';
	}

	public function performDisabledTransformation() {
		$clone = clone $this;
		$clone->setDisabled(true);

		return $clone;
	}

	/**
	 * Transforms the source data for this CheckboxSetField
	 * into a comma separated list of values.
	 *
	 * @return ReadonlyField
	 */
	public function performReadonlyTransformation() {
		$values = '';
		$data = array();

		$items = $this->value;
		if($this->source) {
			foreach($this->source as $source) {
				if(is_object($source)) {
					$sourceTitles[$source->ID] = $source->Title;
foreach ($collection as $item) {
    $myArray['foo'] = $item->getFoo();

    if ($item->hasBar()) {
        $myArray['bar'] = $item->getBar();
    }

    // do something with $myArray
}
				}
			}
		}

		if($items) {
			// Items is a DO Set
			if($items instanceof SS_List) {
				foreach($items as $item) {
					$data[] = $item->Title;
				}
				if($data) $values = implode(', ', $data);


			// Items is an array or single piece of string (including comma seperated string)
			} else {
				if(!is_array($items)) {
					$items = preg_split('/ *, */', trim($items));
				}

				foreach($items as $item) {
					if(is_array($item)) {
						$data[] = $item['Title'];
					} elseif(is_array($this->source) && !empty($this->source[$item])) {
						$data[] = $this->source[$item];
					} elseif(is_a($this->source, 'SS_List')) {
						$data[] = $sourceTitles[$item];
function myFunction($a) {
    switch ($a) {
        case 'foo':
            $x = 1;
            break;

        case 'bar':
            $x = 2;
            break;
    }

    // $x is potentially undefined here.
    echo $x;
}
					} else {
						$data[] = $item;
					}
				}

				$values = implode(', ', $data);
			}
		}

		$field = $this->castedCopy('ReadonlyField');
		$field->setValue($values);

		return $field;
	}

	public function Type() {
		return 'optionset checkboxset';
	}

	public function ExtraOptions() {
		return FormField::ExtraOptions();
abstract class User
{
    /** @return string */
    abstract public function getPassword();
}

class MyUser extends User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}
	}

	/**
	 * Validate this field
	 *
	 * @param Validator $validator
	 * @return bool
	 */
	public function validate($validator) {
		$values = $this->value;
		if (!$values) {
			return true;
		}
		$sourceArray = $this->getSourceAsArray();
		if (is_array($values)) {
			if (!array_intersect_key($sourceArray, $values)) {
				$validator->validationError(
					$this->name,
					_t(
						'CheckboxSetField.SOURCE_VALIDATION',
						"Please select a value within the list provided. '{value}' is not a valid option",
						array('value' => implode(' and ', array_diff($sourceArray, $values)))
function acceptsInteger($int) { }

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

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
					),
					"validation"
				);
				return false;
			}
		} else {

			if (!in_array($this->value, $sourceArray)) {
				$validator->validationError(
					$this->name,
					_t(
						'CheckboxSetField.SOURCE_VALIDATION',
						"Please select a value within the list provided. '{value}' is not a valid option",
						array('value' => $this->value)
function acceptsInteger($int) { }

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

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
					),
					"validation"
				);
				return false;
			}
		}
		return true;
	}

}
