FeatureContext - Code Metrics - Inspection of "Added forumulas validation, enhanced exceptions ha..." - hiqdev/php-billing - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( df1a77...53322f )

by Dmitry

created 2018-06-01 18:21 UTC

FeatureContext A

↳ Parent: Project

Complexity

Total Complexity

Size/Duplication

Total Lines	224
Duplicated Lines	7.14 %

Coupling/Cohesion

Components	1
Dependencies	12

Importance

Changes

Metric	Value
wmc	30
c	0
b	0
f	0
lcom	1
cbo	12
dl	16
loc	224
rs	10

19 Methods

Rating	Name	Duplication	Size	Complexity
A	__construct()	0	5	1
A	formulaIs()	0	4	1
A	priceIs()	8	8	1
A	actionIs()	8	8	1
A	formulaContinues()	0	4	1
A	getFormulaEngine()	0	8	2
A	actionDateIs()	0	4	1
A	multilineErrorIs()	0	4	1
A	errorIs()	0	4	1
A	emptyCharge()	0	4	1
A	chargeWithSum()	0	4	1
A	chargeWithReason()	0	4	1
A	chargeIs()	0	8	2
A	calculateCharges()	0	7	1
A	expectError()	0	15	4
A	isExpectedError()	0	4	1
A	startsWith()	0	4	2
B	assertCharge()	0	15	5
A	ensureNo()	0	10	2

How to fix Duplicated Code

<?php
/**
 * PHP Billing Library
 *
 * @link      https://github.com/hiqdev/php-billing
 * @package   php-billing
 * @license   BSD-3-Clause
 * @copyright Copyright (c) 2017-2018, HiQDev (http://hiqdev.com/)
 */

namespace hiqdev\php\billing\tests\behat\bootstrap;

use Behat\Behat\Context\Context;
use Closure;
use DateTimeImmutable;
use hiqdev\php\billing\action\Action;
use hiqdev\php\billing\charge\Charge;
use hiqdev\php\billing\charge\ChargeInterface;
use hiqdev\php\billing\customer\Customer;
use hiqdev\php\billing\formula\FormulaEngine;
use hiqdev\php\billing\price\SinglePrice;
use hiqdev\php\billing\target\Target;
use hiqdev\php\billing\type\Type;
use hiqdev\php\units\Quantity;
use Money\Currencies\ISOCurrencies;
use Money\Parser\DecimalMoneyParser;
use NumberFormatter;
use PHPUnit\Framework\Assert;

/**
 * Defines application features from the specific context.
 */
class FeatureContext implements Context
{
    protected $engine;

    /** @var Customer */
    protected $customer;
    /**
     * @var \hiqdev\php\billing\price\PriceInterface|\hiqdev\php\billing\charge\FormulaChargeModifierTrait
     *
     * TODO: FormulaChargeModifierTrait::setFormula() must be moved to interface
     */
    protected $price;

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

    /**
     * @var \hiqdev\php\billing\action\ActionInterface|\hiqdev\php\billing\action\AbstractAction
     */
    protected $action;
    /**
     * @var ChargeInterface[]
     */
    protected $charges;

    /** @var \Money\MoneyParser */
    protected $moneyParser;

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

    /**
     * Initializes context.
     */
    public function __construct()
    {
        $this->customer = new Customer(null, 'somebody');
        $this->moneyParser = new DecimalMoneyParser(new ISOCurrencies());
    }

    /**
     * @Given /(\S+) (\S+) price is ([0-9.]+) (\w+) per (\w+)/
     */
    public function priceIs($target, $type, $sum, $currency, $unit)

    {
        $type = new Type(Type::ANY, $type);
        $target = new Target(Target::ANY, $target);
        $quantity = Quantity::create($unit, 0);
        $sum = $this->moneyParser->parse($sum, $currency);
        $this->price = new SinglePrice(null, $type, $target, null, $quantity, $sum);
    }

    /**
     * @Given /action is (\S+) (\w+) ([0-9.]+) (\S+)/
     */
    public function actionIs($target, $type, $amount, $unit)

    {
        $type = new Type(Type::ANY, $type);
        $target = new Target(Target::ANY, $target);
        $quantity = Quantity::create($unit, $amount);
        $time = new DateTimeImmutable();
        $this->action = new Action(null, $type, $target, $quantity, $this->customer, $time);
    }

    /**
     * @Given /formula is (.+)/
     * @param string $formula
     */
    public function formulaIs(string $formula): void
    {
        $this->formula = $formula;
    }

    /**
     * @Given /formula continues (.+)/
     * @param string $formula
     */
    public function formulaContinues(string $formula): void
    {
        $this->formula .= "\n" . $formula;
    }

    protected function getFormulaEngine()
    {
        if ($this->engine === null) {
            $this->engine = new FormulaEngine();
        }

        return $this->engine;
    }

    /**
     * @When /action date is ([0-9.-]+)/
     * @param string $date
     * @throws \Exception
     */
    public function actionDateIs(string $date): void
    {
        $this->action->setTime(new DateTimeImmutable($date));
class A
{
    public function foo() { }
}

class B extends A
{
    public function bar() { }
}

/**
 * @param A|B $x
 */
function someFunction($x)
{
    $x->foo(); // This call is fine as the method exists in A and B.
    $x->bar(); // This method only exists in B and might cause an error.
}
    }

    /**
     * @Then /^error is$/m
     */
    public function multilineErrorIs(\Behat\Gherkin\Node\PyStringNode $value)
    {
        $this->expectedError = $value->getRaw();
    }

    /**
     * @Then /^error is (.+)$/
     *
     * @param string $error
     */
    public function errorIs($error): void
    {
        $this->expectedError = $error;
    }

    /**
     * @Then /^(\w+) charge is ?$/
     * @param string $numeral
     */
    public function emptyCharge(string $numeral): void
    {
        $this->chargeIs($numeral);
    }

    /**
     * @Then /^(\w+) charge is (\S+) +([0-9.]+) ([A-Z]{3})$/
     */
    public function chargeWithSum($numeral, $type = null, $sum = null, $currency = null): void
    {
        $this->chargeIs($numeral, $type, $sum, $currency);
    }

    /**
     * @Then /^(\w+) charge is (\S+) +([0-9.]+) ([A-Z]{3}) reason (.+)/
     */
    public function chargeWithReason($numeral, $type = null, $sum = null, $currency = null, $reason = null): void
    {
        $this->chargeIs($numeral, $type, $sum, $currency, $reason);
    }

    public function chargeIs($numeral, $type = null, $sum = null, $currency = null, $reason = null): void
    {
        $no = $this->ensureNo($numeral);
        if ($no === 0) {
            $this->calculateCharges();
        }
        $this->assertCharge($this->charges[$no] ?? null, $type, $sum, $currency, $reason);
    }

    /**
     * @When /^calculating charges$/
     */
    public function calculateCharges(): void
    {
        $this->expectError(function () {
            $this->price->setFormula($this->getFormulaEngine()->build($this->formula));
class A
{
    public function foo() { }
}

class B extends A
{
    public function bar() { }
}

/**
 * @param A|B $x
 */
function someFunction($x)
{
    $x->foo(); // This call is fine as the method exists in A and B.
    $x->bar(); // This method only exists in B and might cause an error.
}
            $this->charges = $this->price->calculateCharges($this->action);
class A
{
    public function foo() { }
}

class B extends A
{
    public function bar() { }
}

/**
 * @param A|B $x
 */
function someFunction($x)
{
    $x->foo(); // This call is fine as the method exists in A and B.
    $x->bar(); // This method only exists in B and might cause an error.
}
        });
    }

    public function expectError(Closure $closure): void
    {
        try {
            call_user_func($closure);
        } catch (\Exception $e) {
            if ($this->isExpectedError($e)) {
                $this->expectedError = null;
            } else {
                throw $e;
            }
        }
        if ($this->expectedError) {
''   == false // true
''   == null  // true
'ab' == false // false
'ab' == null  // false

// It is often better to use strict comparison
'' === false // false
'' === null  // false
            throw new \Exception('failed receive expected exception');
        }
    }

    protected function isExpectedError(\Exception $e): bool
    {
        return $this->startsWith($e->getMessage(), $this->expectedError);
    }

    protected function startsWith(string $string, string $prefix = null): bool
    {
        return $prefix && strncmp($string, $prefix, strlen($prefix)) === 0;
''   == false // true
''   == null  // true
'ab' == false // false
'ab' == null  // false

// It is often better to use strict comparison
'' === false // false
'' === null  // false
    }

    /**
     * @param ChargeInterface|null $charge
     * @param string|null $type
     * @param string|null $sum
     * @param string|null $currency
     * @param string|null $reason
     */
    public function assertCharge($charge, $type, $sum, $currency, $reason): void
    {
        if (empty($type) && empty($sum) && empty($currency)) {
            Assert::assertNull($charge);

            return;
        }
        Assert::assertInstanceOf(Charge::class, $charge);
        Assert::assertSame($type, $charge->getPrice()->getType()->getName());
function someFunction(A $objectMaybe = null)
{
    if ($objectMaybe instanceof A) {
        $objectMaybe->doSomething();
    }
}
        $money = $this->moneyParser->parse($sum, $currency);
        Assert::assertEquals($money, $charge->getSum()); // TODO: Should we add `getSum()` to ChargeInterface?
        if ($reason !== null) {
            Assert::assertSame($reason, $charge->getComment()); // TODO: Should we add `getComment()` to ChargeInterface?
        }
    }

    private function ensureNo(string $numeral): int
    {
        $formatter = new NumberFormatter('en_EN', NumberFormatter::SPELLOUT);
        $result = $formatter->parse($numeral);
        if ($result === false) {
            throw new \Exception("Wrong numeral '$numeral'");
        }

        return --$result;
    }
}


Push — master ( df1a77...53322f )

FeatureContext A

Complexity

Size/Duplication

Coupling/Cohesion

Importance

19 Methods

How to fix Duplicated Code

Duplicated Code

Available Fixes

Available Fixes

Available Fixes

1		<?php
2		/**
3		* PHP Billing Library
4		*
5		* @link https://github.com/hiqdev/php-billing
6		* @package php-billing
7		* @license BSD-3-Clause
8		* @copyright Copyright (c) 2017-2018, HiQDev (http://hiqdev.com/)
9		*/
10
11		namespace hiqdev\php\billing\tests\behat\bootstrap;
12
13		use Behat\Behat\Context\Context;
14		use Closure;
15		use DateTimeImmutable;
16		use hiqdev\php\billing\action\Action;
17		use hiqdev\php\billing\charge\Charge;
18		use hiqdev\php\billing\charge\ChargeInterface;
19		use hiqdev\php\billing\customer\Customer;
20		use hiqdev\php\billing\formula\FormulaEngine;
21		use hiqdev\php\billing\price\SinglePrice;
22		use hiqdev\php\billing\target\Target;
23		use hiqdev\php\billing\type\Type;
24		use hiqdev\php\units\Quantity;
25		use Money\Currencies\ISOCurrencies;
26		use Money\Parser\DecimalMoneyParser;
27		use NumberFormatter;
28		use PHPUnit\Framework\Assert;
29
30		/**
31		* Defines application features from the specific context.
32		*/
33		class FeatureContext implements Context
34		{
35		protected $engine;
36
37		/** @var Customer */
38		protected $customer;
39		/**
40		* @var \hiqdev\php\billing\price\PriceInterface\|\hiqdev\php\billing\charge\FormulaChargeModifierTrait
41		*
42		* TODO: FormulaChargeModifierTrait::setFormula() must be moved to interface
43		*/
44		protected $price;
45
46		/** @var string */
47		protected $formula;
48
49		/**
50		* @var \hiqdev\php\billing\action\ActionInterface\|\hiqdev\php\billing\action\AbstractAction
51		*/
52		protected $action;
53		/**
54		* @var ChargeInterface[]
55		*/
56		protected $charges;
57
58		/** @var \Money\MoneyParser */
59		protected $moneyParser;
60
61		/** @var string */
62		protected $expectedError;
63
64		/**
65		* Initializes context.
66		*/
67		public function __construct()
68		{
69		$this->customer = new Customer(null, 'somebody');
70		$this->moneyParser = new DecimalMoneyParser(new ISOCurrencies());
71		}
72
73		/**
74		* @Given /(\S+) (\S+) price is ([0-9.]+) (\w+) per (\w+)/
75		*/
76	View Code Duplication	public function priceIs($target, $type, $sum, $currency, $unit)
		0 ignored issues – show Duplication introduced 2018-05-18 16:35 UTC by Report Bug Copy Issue Report This method seems to be duplicated in 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...
77		{
78		$type = new Type(Type::ANY, $type);
79		$target = new Target(Target::ANY, $target);
80		$quantity = Quantity::create($unit, 0);
81		$sum = $this->moneyParser->parse($sum, $currency);
82		$this->price = new SinglePrice(null, $type, $target, null, $quantity, $sum);
83		}
84
85		/**
86		* @Given /action is (\S+) (\w+) ([0-9.]+) (\S+)/
87		*/
88	View Code Duplication	public function actionIs($target, $type, $amount, $unit)
		0 ignored issues – show Duplication introduced 2018-05-18 16:35 UTC by Report Bug Copy Issue Report This method seems to be duplicated in 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...
89		{
90		$type = new Type(Type::ANY, $type);
91		$target = new Target(Target::ANY, $target);
92		$quantity = Quantity::create($unit, $amount);
93		$time = new DateTimeImmutable();
94		$this->action = new Action(null, $type, $target, $quantity, $this->customer, $time);
95		}
96
97		/**
98		* @Given /formula is (.+)/
99		* @param string $formula
100		*/
101		public function formulaIs(string $formula): void
102		{
103		$this->formula = $formula;
104		}
105
106		/**
107		* @Given /formula continues (.+)/
108		* @param string $formula
109		*/
110		public function formulaContinues(string $formula): void
111		{
112		$this->formula .= "\n" . $formula;
113		}
114
115		protected function getFormulaEngine()
116		{
117		if ($this->engine === null) {
118		$this->engine = new FormulaEngine();
119		}
120
121		return $this->engine;
122		}
123
124		/**
125		* @When /action date is ([0-9.-]+)/
126		* @param string $date
127		* @throws \Exception
128		*/
129		public function actionDateIs(string $date): void
130		{
131		$this->action->setTime(new DateTimeImmutable($date));
		0 ignored issues – show Bug introduced 2018-05-17 16:09 UTC by Report Bug Copy Issue Report The method `setTime` does only exist in `hiqdev\php\billing\action\AbstractAction`, but not in `hiqdev\php\billing\action\ActionInterface`. It seems like the method you are trying to call exists only in some of the possible types. Let’s take a look at an example: class A { public function foo() { } } class B extends A { public function bar() { } } /** * @param A\|B $x / function someFunction($x) { $x->foo(); // This call is fine as the method exists in A and B. $x->bar(); // This method only exists in B and might cause an error. } Available Fixes Add an additional type-check: /* * @param A\|B $x / function someFunction($x) { $x->foo(); if ($x instanceof B) { $x->bar(); } } Only allow a single type to be passed if the variable comes from a parameter: function someFunction(B $x) { /* ... */ } Loading history...
132		}
133
134		/**
135		* @Then /^error is$/m
136		*/
137		public function multilineErrorIs(\Behat\Gherkin\Node\PyStringNode $value)
138		{
139		$this->expectedError = $value->getRaw();
140		}
141
142		/**
143		* @Then /^error is (.+)$/
144		*
145		* @param string $error
146		*/
147		public function errorIs($error): void
148		{
149		$this->expectedError = $error;
150		}
151
152		/**
153		* @Then /^(\w+) charge is ?$/
154		* @param string $numeral
155		*/
156		public function emptyCharge(string $numeral): void
157		{
158		$this->chargeIs($numeral);
159		}
160
161		/**
162		* @Then /^(\w+) charge is (\S+) +([0-9.]+) ([A-Z]{3})$/
163		*/
164		public function chargeWithSum($numeral, $type = null, $sum = null, $currency = null): void
165		{
166		$this->chargeIs($numeral, $type, $sum, $currency);
167		}
168
169		/**
170		* @Then /^(\w+) charge is (\S+) +([0-9.]+) ([A-Z]{3}) reason (.+)/
171		*/
172		public function chargeWithReason($numeral, $type = null, $sum = null, $currency = null, $reason = null): void
173		{
174		$this->chargeIs($numeral, $type, $sum, $currency, $reason);
175		}
176
177		public function chargeIs($numeral, $type = null, $sum = null, $currency = null, $reason = null): void
178		{
179		$no = $this->ensureNo($numeral);
180		if ($no === 0) {
181		$this->calculateCharges();
182		}
183		$this->assertCharge($this->charges[$no] ?? null, $type, $sum, $currency, $reason);
184		}
185
186		/**
187		* @When /^calculating charges$/
188		*/
189		public function calculateCharges(): void
190		{
191		$this->expectError(function () {
192		$this->price->setFormula($this->getFormulaEngine()->build($this->formula));
		0 ignored issues – show Bug introduced 2018-05-17 16:09 UTC by Report Bug Copy Issue Report The method `setFormula` does only exist in `hiqdev\php\billing\charg...mulaChargeModifierTrait`, but not in `hiqdev\php\billing\price\PriceInterface`. It seems like the method you are trying to call exists only in some of the possible types. Let’s take a look at an example: class A { public function foo() { } } class B extends A { public function bar() { } } /** * @param A\|B $x / function someFunction($x) { $x->foo(); // This call is fine as the method exists in A and B. $x->bar(); // This method only exists in B and might cause an error. } Available Fixes Add an additional type-check: /* * @param A\|B $x / function someFunction($x) { $x->foo(); if ($x instanceof B) { $x->bar(); } } Only allow a single type to be passed if the variable comes from a parameter: function someFunction(B $x) { /* ... */ } Loading history...
193		$this->charges = $this->price->calculateCharges($this->action);
		0 ignored issues – show Bug introduced 2018-05-17 16:09 UTC by Report Bug Copy Issue Report The method `calculateCharges` does only exist in `hiqdev\php\billing\price\PriceInterface`, but not in `hiqdev\php\billing\charg...mulaChargeModifierTrait`. It seems like the method you are trying to call exists only in some of the possible types. Let’s take a look at an example: class A { public function foo() { } } class B extends A { public function bar() { } } /** * @param A\|B $x / function someFunction($x) { $x->foo(); // This call is fine as the method exists in A and B. $x->bar(); // This method only exists in B and might cause an error. } Available Fixes Add an additional type-check: /* * @param A\|B $x / function someFunction($x) { $x->foo(); if ($x instanceof B) { $x->bar(); } } Only allow a single type to be passed if the variable comes from a parameter: function someFunction(B $x) { /* ... */ } Loading history...
194		});
195		}
196
197		public function expectError(Closure $closure): void
198		{
199		try {
200		call_user_func($closure);
201		} catch (\Exception $e) {
202		if ($this->isExpectedError($e)) {
203		$this->expectedError = null;
204		} else {
205		throw $e;
206		}
207		}
208		if ($this->expectedError) {
		0 ignored issues – show Bug Best Practice introduced 2018-05-23 08:23 UTC by Report Bug Copy Issue Report The expression `$this->expectedError` of type `null\|string` is loosely compared to `true`; 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...
209		throw new \Exception('failed receive expected exception');
210		}
211		}
212
213		protected function isExpectedError(\Exception $e): bool
214		{
215		return $this->startsWith($e->getMessage(), $this->expectedError);
216		}
217
218		protected function startsWith(string $string, string $prefix = null): bool
219		{
220		return $prefix && strncmp($string, $prefix, strlen($prefix)) === 0;
		0 ignored issues – show Bug Best Practice introduced 2018-05-23 08:23 UTC by Report Bug Copy Issue Report The expression `$prefix` of type `null\|string` is loosely compared to `true`; 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...
221		}
222
223		/**
224		* @param ChargeInterface\|null $charge
225		* @param string\|null $type
226		* @param string\|null $sum
227		* @param string\|null $currency
228		* @param string\|null $reason
229		*/
230		public function assertCharge($charge, $type, $sum, $currency, $reason): void
231		{
232		if (empty($type) && empty($sum) && empty($currency)) {
233		Assert::assertNull($charge);
234
235		return;
236		}
237		Assert::assertInstanceOf(Charge::class, $charge);
238		Assert::assertSame($type, $charge->getPrice()->getType()->getName());
		0 ignored issues – show Bug introduced 2018-05-17 16:09 UTC by Report Bug Copy Issue Report It seems like `$charge` is not always an object, but can also be of type `null`. Maybe add an additional type check? If a variable is not always an object, we recommend to add an additional type check to ensure your method call is safe: function someFunction(A $objectMaybe = null) { if ($objectMaybe instanceof A) { $objectMaybe->doSomething(); } } Loading history...
239		$money = $this->moneyParser->parse($sum, $currency);
240		Assert::assertEquals($money, $charge->getSum()); // TODO: Should we add `getSum()` to ChargeInterface?
241		if ($reason !== null) {
242		Assert::assertSame($reason, $charge->getComment()); // TODO: Should we add `getComment()` to ChargeInterface?
243		}
244		}
245
246		private function ensureNo(string $numeral): int
247		{
248		$formatter = new NumberFormatter('en_EN', NumberFormatter::SPELLOUT);
249		$result = $formatter->parse($numeral);
250		if ($result === false) {
251		throw new \Exception("Wrong numeral '$numeral'");
252		}
253
254		return --$result;
255		}
256		}
257

hiqdev / php-billing

Push — master ( df1a77...53322f )

FeatureContext A

Complexity

Size/Duplication

Coupling/Cohesion

Importance

19 Methods

How to fix Duplicated Code

Duplicated Code

Available Fixes

Available Fixes

Available Fixes

Duplication Side-by-Side

Filter issues like