WC_Item_Product - Code Metrics - Inspection of "2.7 - Discount calculation classes" - woothemes/woocommerce - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Pull Request — master (#11889)

by Mike

created 2016-09-13 17:27 UTC

WC_Item_Product B

↳ Parent: Project

Complexity

Total Complexity

Size/Duplication

Total Lines	247
Duplicated Lines	1.21 %

Coupling/Cohesion

Components	1
Dependencies	3

Importance

Changes

Metric	Value
dl	3
loc	247
rs	8.295
c	0
b	0
f	0
wmc	42
lcom	1
cbo	3

21 Methods

Rating	Name	Duplication	Size	Complexity
A	offsetGet()	0	7	3
A	offsetSet()	0	10	2
A	offsetExists()	3	6	3
A	offsetUnset()	0	3	1
A	get_product()	0	3	3
A	get_price()	0	3	2
A	get_weight()	0	3	2
A	get_tax_status()	0	3	2
A	set_name()	0	3	1
A	set_quantity()	0	6	2
A	set_tax_class()	0	6	3
A	set_product_id()	0	6	3
A	set_variation_id()	0	6	3
A	set_variation()	0	5	2
A	set_product()	0	11	4
A	get_name()	0	3	1
A	get_type()	0	3	1
A	get_product_id()	0	3	1
A	get_variation_id()	0	3	1
A	get_quantity()	0	3	1
A	get_tax_class()	0	3	1

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

<?php
if ( ! defined( 'ABSPATH' ) ) {
	exit;
}

/**
 * Line Item.
 *
 * @version     2.7.0
 * @since       2.7.0
 * @package     WooCommerce/Classes
 * @author      WooThemes
 */
class WC_Item_Product extends WC_Item {

	/**
	 * Data array.
	 * @since 2.7.0
	 * @var array
	 */
	protected $data = array(
		'name'         => '',
		'product_id'   => 0,
		'variation_id' => 0,
		'quantity'     => 1,
	);

	/**
	 * Product this item represents.
	 * @var WC_Product
	 */
	protected $product = null;

	/**
	 * offsetGet for ArrayAccess/Backwards compatibility.
	 * @deprecated Add deprecation notices in future release.
	 * @param string $offset
	 * @return mixed
	 */
	public function offsetGet( $offset ) {
		switch ( $offset ) {
			case 'data' :
				return $this->get_product();
		}
		return isset( $this->data[ $offset ] ) ? $this->data[ $offset ] : '';
	}

	/**
	 * offsetSet for ArrayAccess/Backwards compatibility.
	 * @deprecated Add deprecation notices in future release.
	 * @param string $offset
	 * @param mixed $value
	 */
	public function offsetSet( $offset, $value ) {
		switch ( $offset ) {
			case 'data' :
				$this->set_product( $value );
				break;
			default :
				$this->data[ $offset ] = $value;
				break;
		}
	}

	/**
	 * offsetExists for ArrayAccess
	 * @param string $offset
	 * @return bool
	 */
	public function offsetExists( $offset ) {
		if ( in_array( $offset, array( 'data' ) ) || isset( $this->data[ $offset ] ) ) {

			return true;
		}
		return false;
	}

	/**
	 * offsetUnset for ArrayAccess
	 * @param string $offset
	 */
	public function offsetUnset( $offset ) {
		unset( $this->data[ $offset ] );
	}

	/**
	 * Get product object.
	 * @return WC_Product
	 */
	public function get_product() {
		return ! is_null( $this->product ) ? $this->product : ( $this->product = wc_get_product( $this->get_variation_id() ? $this->get_variation_id() : $this->get_product_id() ) );
	}

	/**
	 * Gets price of the product.
	 * @return float
	 */
	public function get_price() {
		return $this->get_product() ? $this->get_product()->get_price() : 0;
	}

	/**
	 * Gets price of the product.
	 * @return float
	 */
	public function get_weight() {
		return $this->get_product() ? $this->get_product()->get_weight() : 0;
	}

	/**
	 * Get tax status.
	 * @return string
	 */
	public function get_tax_status() {
		return $this->get_product() ? $this->get_product()->get_tax_status() : 'taxable';
	}

	/*
	|--------------------------------------------------------------------------
	| Setters
	|--------------------------------------------------------------------------
	*/

	/**
	 * Set order item name.
	 * @param string $value
	 * @throws WC_Data_Exception
	 */
	public function set_name( $value ) {
		$this->data['name'] = wc_clean( $value );
	}

	/**
	 * Set quantity.
	 * @param int $value
	 * @throws WC_Data_Exception
	 */
	public function set_quantity( $value ) {
		if ( 0 >= $value ) {
			$this->error( 'line_item_product_invalid_quantity', __( 'Quantity must be positive', 'woocommerce' ) );
		}
		$this->data['quantity'] = wc_stock_amount( $value );
	}

	/**
	 * Set tax class.
	 * @param string $value
	 * @throws WC_Data_Exception
	 */
	public function set_tax_class( $value ) {
		if ( $value && ! in_array( $value, WC_Tax::get_tax_classes() ) ) {
			$this->error( 'line_item_product_invalid_tax_class', __( 'Invalid tax class', 'woocommerce' ) );
		}
		$this->data['tax_class'] = $value;
	}

	/**
	 * Set Product ID
	 * @param int $value
	 * @throws WC_Data_Exception
	 */
	public function set_product_id( $value ) {
		if ( $value > 0 && 'product' !== get_post_type( absint( $value ) ) ) {
			$this->error( 'line_item_product_invalid_product_id', __( 'Invalid product ID', 'woocommerce' ) );
		}
		$this->data['product_id'] = absint( $value );
	}

	/**
	 * Set variation ID.
	 * @param int $value
	 * @throws WC_Data_Exception
	 */
	public function set_variation_id( $value ) {
		if ( $value > 0 && 'product_variation' !== get_post_type( $value ) ) {
			$this->error( 'line_item_product_invalid_variation_id', __( 'Invalid variation ID', 'woocommerce' ) );
		}
		$this->data['variation_id'] = absint( $value );
	}

	/**
	 * Set variation data (stored as meta data - write only).
	 * @param array $data Key/Value pairs
	 */
	public function set_variation( $data ) {
		foreach ( $data as $key => $value ) {
			$this->add_meta_data( str_replace( 'attribute_', '', $key ), $value, true );
		}
	}

	/**
	 * Set properties based on passed in product object.
	 * @param WC_Product $product
	 * @throws WC_Data_Exception
	 */
	public function set_product( $product ) {
		if ( ! is_a( $product, 'WC_Product' ) ) {
			$this->error( 'line_item_product_invalid_product', __( 'Invalid product', 'woocommerce' ) );
		}
		$this->product = $product;
		$this->set_product_id( $product->get_id() );
		$this->set_name( $product->get_title() );
		$this->set_tax_class( $product->get_tax_class() );
		$this->set_variation_id( is_callable( array( $product, 'get_variation_id' ) ) ? $product->get_variation_id() : 0 );
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.
    }
}
		$this->set_variation( is_callable( array( $product, 'get_variation_attributes' ) ) ? $product->get_variation_attributes() : array() );
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.
    }
}
	}

	/*
	|--------------------------------------------------------------------------
	| Getters
	|--------------------------------------------------------------------------
	*/

	/**
	 * Get order item name.
	 * @return string
	 */
	public function get_name() {
		return $this->data['name'];
	}

	/**
	 * Get item type.
	 * @return string
	 */
	public function get_type() {
		return 'line_item';
	}

	/**
	 * Get product ID.
	 * @return int
	 */
	public function get_product_id() {
		return absint( $this->data['product_id'] );
	}

	/**
	 * Get variation ID.
	 * @return int
	 */
	public function get_variation_id() {
		return absint( $this->data['variation_id'] );
	}

	/**
	 * Get quantity.
	 * @return int
	 */
	public function get_quantity() {
		return wc_stock_amount( $this->data['quantity'] );
	}

	/**
	 * Get tax class.
	 * @return string
	 */
	public function get_tax_class() {
		return $this->data['tax_class'];
	}
}


Pull Request — master (#11889)

WC_Item_Product B

Complexity

Size/Duplication

Coupling/Cohesion

Importance

21 Methods

How to fix Duplicated Code Complexity

Duplicated Code

Complex Class

Available Fixes

Available Fixes

1		<?php
2		if ( ! defined( 'ABSPATH' ) ) {
3		exit;
4		}
5
6		/**
7		* Line Item.
8		*
9		* @version 2.7.0
10		* @since 2.7.0
11		* @package WooCommerce/Classes
12		* @author WooThemes
13		*/
14		class WC_Item_Product extends WC_Item {
15
16		/**
17		* Data array.
18		* @since 2.7.0
19		* @var array
20		*/
21		protected $data = array(
22		'name' => '',
23		'product_id' => 0,
24		'variation_id' => 0,
25		'quantity' => 1,
26		);
27
28		/**
29		* Product this item represents.
30		* @var WC_Product
31		*/
32		protected $product = null;
33
34		/**
35		* offsetGet for ArrayAccess/Backwards compatibility.
36		* @deprecated Add deprecation notices in future release.
37		* @param string $offset
38		* @return mixed
39		*/
40		public function offsetGet( $offset ) {
41		switch ( $offset ) {
42		case 'data' :
43		return $this->get_product();
44		}
45		return isset( $this->data[ $offset ] ) ? $this->data[ $offset ] : '';
46		}
47
48		/**
49		* offsetSet for ArrayAccess/Backwards compatibility.
50		* @deprecated Add deprecation notices in future release.
51		* @param string $offset
52		* @param mixed $value
53		*/
54		public function offsetSet( $offset, $value ) {
55		switch ( $offset ) {
56		case 'data' :
57		$this->set_product( $value );
58		break;
59		default :
60		$this->data[ $offset ] = $value;
61		break;
62		}
63		}
64
65		/**
66		* offsetExists for ArrayAccess
67		* @param string $offset
68		* @return bool
69		*/
70		public function offsetExists( $offset ) {
71	View Code Duplication	if ( in_array( $offset, array( 'data' ) ) \|\| isset( $this->data[ $offset ] ) ) {
		0 ignored issues – show Duplication introduced 2016-09-13 12:56 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...
72		return true;
73		}
74		return false;
75		}
76
77		/**
78		* offsetUnset for ArrayAccess
79		* @param string $offset
80		*/
81		public function offsetUnset( $offset ) {
82		unset( $this->data[ $offset ] );
83		}
84
85		/**
86		* Get product object.
87		* @return WC_Product
88		*/
89		public function get_product() {
90		return ! is_null( $this->product ) ? $this->product : ( $this->product = wc_get_product( $this->get_variation_id() ? $this->get_variation_id() : $this->get_product_id() ) );
91		}
92
93		/**
94		* Gets price of the product.
95		* @return float
96		*/
97		public function get_price() {
98		return $this->get_product() ? $this->get_product()->get_price() : 0;
99		}
100
101		/**
102		* Gets price of the product.
103		* @return float
104		*/
105		public function get_weight() {
106		return $this->get_product() ? $this->get_product()->get_weight() : 0;
107		}
108
109		/**
110		* Get tax status.
111		* @return string
112		*/
113		public function get_tax_status() {
114		return $this->get_product() ? $this->get_product()->get_tax_status() : 'taxable';
115		}
116
117		/*
118		\|--------------------------------------------------------------------------
119		\| Setters
120		\|--------------------------------------------------------------------------
121		*/
122
123		/**
124		* Set order item name.
125		* @param string $value
126		* @throws WC_Data_Exception
127		*/
128		public function set_name( $value ) {
129		$this->data['name'] = wc_clean( $value );
130		}
131
132		/**
133		* Set quantity.
134		* @param int $value
135		* @throws WC_Data_Exception
136		*/
137		public function set_quantity( $value ) {
138		if ( 0 >= $value ) {
139		$this->error( 'line_item_product_invalid_quantity', __( 'Quantity must be positive', 'woocommerce' ) );
140		}
141		$this->data['quantity'] = wc_stock_amount( $value );
142		}
143
144		/**
145		* Set tax class.
146		* @param string $value
147		* @throws WC_Data_Exception
148		*/
149		public function set_tax_class( $value ) {
150		if ( $value && ! in_array( $value, WC_Tax::get_tax_classes() ) ) {
151		$this->error( 'line_item_product_invalid_tax_class', __( 'Invalid tax class', 'woocommerce' ) );
152		}
153		$this->data['tax_class'] = $value;
154		}
155
156		/**
157		* Set Product ID
158		* @param int $value
159		* @throws WC_Data_Exception
160		*/
161		public function set_product_id( $value ) {
162		if ( $value > 0 && 'product' !== get_post_type( absint( $value ) ) ) {
163		$this->error( 'line_item_product_invalid_product_id', __( 'Invalid product ID', 'woocommerce' ) );
164		}
165		$this->data['product_id'] = absint( $value );
166		}
167
168		/**
169		* Set variation ID.
170		* @param int $value
171		* @throws WC_Data_Exception
172		*/
173		public function set_variation_id( $value ) {
174		if ( $value > 0 && 'product_variation' !== get_post_type( $value ) ) {
175		$this->error( 'line_item_product_invalid_variation_id', __( 'Invalid variation ID', 'woocommerce' ) );
176		}
177		$this->data['variation_id'] = absint( $value );
178		}
179
180		/**
181		* Set variation data (stored as meta data - write only).
182		* @param array $data Key/Value pairs
183		*/
184		public function set_variation( $data ) {
185		foreach ( $data as $key => $value ) {
186		$this->add_meta_data( str_replace( 'attribute_', '', $key ), $value, true );
187		}
188		}
189
190		/**
191		* Set properties based on passed in product object.
192		* @param WC_Product $product
193		* @throws WC_Data_Exception
194		*/
195		public function set_product( $product ) {
196		if ( ! is_a( $product, 'WC_Product' ) ) {
197		$this->error( 'line_item_product_invalid_product', __( 'Invalid product', 'woocommerce' ) );
198		}
199		$this->product = $product;
200		$this->set_product_id( $product->get_id() );
201		$this->set_name( $product->get_title() );
202		$this->set_tax_class( $product->get_tax_class() );
203		$this->set_variation_id( is_callable( array( $product, 'get_variation_id' ) ) ? $product->get_variation_id() : 0 );
		0 ignored issues – show Bug introduced 2016-09-13 12:56 UTC by Report Bug Copy Issue Report It seems like you code against a specific sub-type and not the parent class `WC_Product` as the method `get_variation_id()` does only exist in the following sub-classes of `WC_Product`: `WC_Product_Variation`. Maybe you want to `instanceof` check for one of these explicitly? Let’s take a look at an example: 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. } } In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different sub-classes of User which does not have a getDisplayName() method, the code will break. Available Fixes Change the type-hint for the parameter: class AuthSystem { public function authenticate(MyUser $user) { / ... / } } Add an additional type-check: class AuthSystem { public function authenticate(User $user) { if ($user instanceof MyUser) { $this->logger->info(/* ... /); } // or alternatively if ( ! $user instanceof MyUser) { throw new \LogicException( '$user must be an instance of MyUser, ' .'other instances are not supported.' ); } } } Note:* PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case. Add the method to the parent class: abstract class User { /** @return string / abstract public function getPassword(); /* @return string */ abstract public function getDisplayName(); } Loading history...
204		$this->set_variation( is_callable( array( $product, 'get_variation_attributes' ) ) ? $product->get_variation_attributes() : array() );
		0 ignored issues – show Bug introduced 2016-09-13 12:56 UTC by Report Bug Copy Issue Report It seems like you code against a specific sub-type and not the parent class `WC_Product` as the method `get_variation_attributes()` does only exist in the following sub-classes of `WC_Product`: `WC_Product_Variable`, `WC_Product_Variation`. Maybe you want to `instanceof` check for one of these explicitly? Let’s take a look at an example: 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. } } In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different sub-classes of User which does not have a getDisplayName() method, the code will break. Available Fixes Change the type-hint for the parameter: class AuthSystem { public function authenticate(MyUser $user) { / ... / } } Add an additional type-check: class AuthSystem { public function authenticate(User $user) { if ($user instanceof MyUser) { $this->logger->info(/* ... /); } // or alternatively if ( ! $user instanceof MyUser) { throw new \LogicException( '$user must be an instance of MyUser, ' .'other instances are not supported.' ); } } } Note:* PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case. Add the method to the parent class: abstract class User { /** @return string / abstract public function getPassword(); /* @return string */ abstract public function getDisplayName(); } Loading history...
205		}
206
207		/*
208		\|--------------------------------------------------------------------------
209		\| Getters
210		\|--------------------------------------------------------------------------
211		*/
212
213		/**
214		* Get order item name.
215		* @return string
216		*/
217		public function get_name() {
218		return $this->data['name'];
219		}
220
221		/**
222		* Get item type.
223		* @return string
224		*/
225		public function get_type() {
226		return 'line_item';
227		}
228
229		/**
230		* Get product ID.
231		* @return int
232		*/
233		public function get_product_id() {
234		return absint( $this->data['product_id'] );
235		}
236
237		/**
238		* Get variation ID.
239		* @return int
240		*/
241		public function get_variation_id() {
242		return absint( $this->data['variation_id'] );
243		}
244
245		/**
246		* Get quantity.
247		* @return int
248		*/
249		public function get_quantity() {
250		return wc_stock_amount( $this->data['quantity'] );
251		}
252
253		/**
254		* Get tax class.
255		* @return string
256		*/
257		public function get_tax_class() {
258		return $this->data['tax_class'];
259		}
260		}
261

woothemes / woocommerce

Pull Request — master (#11889)

WC_Item_Product B

Complexity

Size/Duplication

Coupling/Cohesion

Importance

21 Methods

How to fix Duplicated Code Complexity

Duplicated Code

Complex Class

Available Fixes

Available Fixes

Duplication Side-by-Side

Filter issues like