HasManyList::remove() - Code Metrics - Inspection of "Added 3.7.1-rc1 changelog" - silverstripe/silverstripe-framework - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — 3.7 ( 81b2d8...ef0909 )

unknown

created 2018-06-24 23:46 UTC

HasManyList::remove() A

↳ Parent: HasManyList

Complexity

Conditions	6
Paths	3

Size

Total Lines

Duplication

Lines	0
Ratio	0 %

Importance

Changes

Metric	Value
cc	6
nc	3
nop	1
dl	0
loc	19
rs	9.0111
c	0
b	0
f	0

<?php

/**
 * Subclass of {@link DataList} representing a has_many relation.
 *
 * @package framework
 * @subpackage model
 */
class HasManyList extends RelationList {

	/**
	 * @var string
	 */
	protected $foreignKey;

	/**
	 * Create a new HasManyList object.
	 * Generation of the appropriate record set is left up to the caller, using the normal
	 * {@link DataList} methods.  Addition arguments are used to support {@@link add()}
	 * and {@link remove()} methods.
	 *
	 * @param string $dataClass The class of the DataObjects that this will list.
	 * @param string $foreignKey The name of the foreign key field to set the ID filter against.
	 */
	public function __construct($dataClass, $foreignKey) {
		parent::__construct($dataClass);

		$this->foreignKey = $foreignKey;
	}

	/**
	 * Gets the field name which holds the related object ID.
	 *
	 * @return string
	 */
	public function getForeignKey() {
		return $this->foreignKey;
	}

	/**
	 * @param null|int $id
	 * @return array
	 */
	protected function foreignIDFilter($id = null) {
		if ($id === null) $id = $this->getForeignID();

		// Try to include the table name for the given foreign key
		if ($table = ClassInfo::table_for_object_field($this->dataClass, $this->foreignKey)) {
			$key = "\"$table\".\"$this->foreignKey\"";
		} else {
			$key = "\"$this->foreignKey\"";
		}

		// Apply relation filter
		if(is_array($id)) {
			return array("$key IN (".DB::placeholders($id).")"  => $id);
		} else if($id !== null){
			return array($key => $id);
		}
	}

	/**
	 * Adds the item to this relation.
	 *
	 * It does so by setting the relationFilters.
	 *
	 * @param DataObject|int $item The DataObject to be added, or its ID
	 */
	public function add($item) {
		if(is_numeric($item)) {
			$item = DataObject::get_by_id($this->dataClass, $item);
		} else if(!($item instanceof $this->dataClass)) {
			user_error("HasManyList::add() expecting a $this->dataClass object, or ID value", E_USER_ERROR);
		}

		$foreignID = $this->getForeignID();

		// Validate foreignID
		if(!$foreignID) {
''   == false // true
''   == null  // true
'ab' == false // false
'ab' == null  // false

// It is often better to use strict comparison
'' === false // false
'' === null  // false
			user_error("ManyManyList::add() can't be called until a foreign ID is set", E_USER_WARNING);
			return;
		}
		if(is_array($foreignID)) {
			user_error("ManyManyList::add() can't be called on a list linked to mulitple foreign IDs", E_USER_WARNING);
			return;
		}

		$foreignKey = $this->foreignKey;
		$item->$foreignKey = $foreignID;

		$item->write();
	}

	/**
	 * Remove an item from this relation.
	 *
	 * Doesn't actually remove the item, it just clears the foreign key value.
	 *
	 * @param int $itemID The ID of the item to be removed.
	 */
	public function removeByID($itemID) {
		$item = $this->byID($itemID);

		return $this->remove($item);
/** @return stdClass|null */
function mayReturnNull() { }

function doesNotAcceptNull(stdClass $x) { }

// With potential error.
function withoutCheck() {
    $x = mayReturnNull();
    doesNotAcceptNull($x); // Potential error here.
}

// Safe - Alternative 1
function withCheck1() {
    $x = mayReturnNull();
    if ( ! $x instanceof stdClass) {
        throw new \LogicException('$x must be defined.');
    }
    doesNotAcceptNull($x);
}

// Safe - Alternative 2
function withCheck2() {
    $x = mayReturnNull();
    if ($x instanceof stdClass) {
        doesNotAcceptNull($x);
    }
}
	}

	/**
	 * Remove an item from this relation.
	 * Doesn't actually remove the item, it just clears the foreign key value.
	 *
	 * @param DataObject $item The DataObject to be removed
	 * @todo Maybe we should delete the object instead?
	 */
	public function remove($item) {
		if(!($item instanceof $this->dataClass)) {
			throw new InvalidArgumentException("HasManyList::remove() expecting a $this->dataClass object, or ID",
				E_USER_ERROR);
		}

		// Don't remove item which doesn't belong to this list
		$foreignID = $this->getForeignID();
		$foreignKey = $this->getForeignKey();

		if(	empty($foreignID)
			|| (is_array($foreignID) && in_array($item->$foreignKey, $foreignID))
			|| $foreignID == $item->$foreignKey
		) {
			$item->$foreignKey = null;
			$item->write();
		}

	}
}


1			<?php
2
3			/**
4			* Subclass of {@link DataList} representing a has_many relation.
5			*
6			* @package framework
7			* @subpackage model
8			*/
9			class HasManyList extends RelationList {
10
11			/**
12			* @var string
13			*/
14			protected $foreignKey;
15
16			/**
17			* Create a new HasManyList object.
18			* Generation of the appropriate record set is left up to the caller, using the normal
19			* {@link DataList} methods. Addition arguments are used to support {@@link add()}
20			* and {@link remove()} methods.
21			*
22			* @param string $dataClass The class of the DataObjects that this will list.
23			* @param string $foreignKey The name of the foreign key field to set the ID filter against.
24			*/
25			public function __construct($dataClass, $foreignKey) {
26			parent::__construct($dataClass);
27
28			$this->foreignKey = $foreignKey;
29			}
30
31			/**
32			* Gets the field name which holds the related object ID.
33			*
34			* @return string
35			*/
36			public function getForeignKey() {
37			return $this->foreignKey;
38			}
39
40			/**
41			* @param null\|int $id
42			* @return array
43			*/
44			protected function foreignIDFilter($id = null) {
45			if ($id === null) $id = $this->getForeignID();
46
47			// Try to include the table name for the given foreign key
48			if ($table = ClassInfo::table_for_object_field($this->dataClass, $this->foreignKey)) {
49			$key = "\"$table\".\"$this->foreignKey\"";
50			} else {
51			$key = "\"$this->foreignKey\"";
52			}
53
54			// Apply relation filter
55			if(is_array($id)) {
56			return array("$key IN (".DB::placeholders($id).")" => $id);
57			} else if($id !== null){
58			return array($key => $id);
59			}
60			}
61
62			/**
63			* Adds the item to this relation.
64			*
65			* It does so by setting the relationFilters.
66			*
67			* @param DataObject\|int $item The DataObject to be added, or its ID
68			*/
69			public function add($item) {
70			if(is_numeric($item)) {
71			$item = DataObject::get_by_id($this->dataClass, $item);
72			} else if(!($item instanceof $this->dataClass)) {
73			user_error("HasManyList::add() expecting a $this->dataClass object, or ID value", E_USER_ERROR);
74			}
75
76			$foreignID = $this->getForeignID();
77
78			// Validate foreignID
79			if(!$foreignID) {
			0 ignored issues – show Bug Best Practice introduced 2016-06-23 23:55 UTC by Report Bug Copy Issue Report The expression `$foreignID` of type `string\|null` is loosely compared to `false`; this is ambiguous if the string can be empty. You might want to explicitly use `=== null` instead. In PHP, under loose comparison (like `==`, or `!=`, or `switch` conditions), values of different types might be equal. For `string` values, the empty string `''` is a special case, in particular the following results might be unexpected: '' == false // true '' == null // true 'ab' == false // false 'ab' == null // false // It is often better to use strict comparison '' === false // false '' === null // false Loading history...
80			user_error("ManyManyList::add() can't be called until a foreign ID is set", E_USER_WARNING);
81			return;
82			}
83			if(is_array($foreignID)) {
84			user_error("ManyManyList::add() can't be called on a list linked to mulitple foreign IDs", E_USER_WARNING);
85			return;
86			}
87
88			$foreignKey = $this->foreignKey;
89			$item->$foreignKey = $foreignID;
90
91			$item->write();
92			}
93
94			/**
95			* Remove an item from this relation.
96			*
97			* Doesn't actually remove the item, it just clears the foreign key value.
98			*
99			* @param int $itemID The ID of the item to be removed.
100			*/
101			public function removeByID($itemID) {
102			$item = $this->byID($itemID);
103
104			return $this->remove($item);
			0 ignored issues – show Bug introduced 2016-06-02 04:12 UTC by Report Bug Copy Issue Report It seems like `$item` defined by `$this->byID($itemID)` on line `102` can be `null`; however, `HasManyList::remove()` does not accept `null`, maybe add an additional type check? Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code: /** @return stdClass\|null */ function mayReturnNull() { } function doesNotAcceptNull(stdClass $x) { } // With potential error. function withoutCheck() { $x = mayReturnNull(); doesNotAcceptNull($x); // Potential error here. } // Safe - Alternative 1 function withCheck1() { $x = mayReturnNull(); if ( ! $x instanceof stdClass) { throw new \LogicException('$x must be defined.'); } doesNotAcceptNull($x); } // Safe - Alternative 2 function withCheck2() { $x = mayReturnNull(); if ($x instanceof stdClass) { doesNotAcceptNull($x); } } Loading history...
105			}
106
107			/**
108			* Remove an item from this relation.
109			* Doesn't actually remove the item, it just clears the foreign key value.
110			*
111			* @param DataObject $item The DataObject to be removed
112			* @todo Maybe we should delete the object instead?
113			*/
114			public function remove($item) {
115			if(!($item instanceof $this->dataClass)) {
116			throw new InvalidArgumentException("HasManyList::remove() expecting a $this->dataClass object, or ID",
117			E_USER_ERROR);
118			}
119
120			// Don't remove item which doesn't belong to this list
121			$foreignID = $this->getForeignID();
122			$foreignKey = $this->getForeignKey();
123
124			if( empty($foreignID)
125			\|\| (is_array($foreignID) && in_array($item->$foreignKey, $foreignID))
126			\|\| $foreignID == $item->$foreignKey
127			) {
128			$item->$foreignKey = null;
129			$item->write();
130			}
131
132			}
133			}
134

silverstripe / silverstripe-framework

Push — 3.7 ( 81b2d8...ef0909 )

HasManyList::remove() A

Complexity

Size

Duplication

Importance

Duplication Side-by-Side

Filter issues like