CountryURLProvider::replaceCountryCodeInUrl() - Code Metrics - Inspection of "PATCH: refactoring of country switcher, to deal be..." - sunnysideup/silverstripe-ecommerce_countries - Measure and Improve Code Quality continuously with Scrutinizer

Passed

Push — master ( 244f37...eeee86 )

by Nicolaas

created 2018-02-14 08:33 UTC

CountryURLProvider::replaceCountryCodeInUrl() B

↳ Parent: CountryURLProvider

Complexity

Conditions	6
Paths	10

Size

Total Lines	35
Code Lines	26

Duplication

Lines	0
Ratio	0 %

Importance

Changes

Metric	Value
dl	0
loc	35
rs	8.439
c	0
b	0
f	0
cc	6
eloc	26
nc	10
nop	2

<?php

/**
 * Usage:
 *     $myAnswer =
 *         CountryPrice_Translation::get_country_url_provider()
 *             ->getSomething();
 *
 */

class CountryURLProvider extends Object implements CountryURLProviderInterface
namespace YourVendor;

class YourClass { }
{


    /**
     * returns the selected country code if there is one ...
     * as an uppercase code, e.g. NZ
     * @param string|null $url
     *
     * @return bool
     */
    public function hasCountrySegment($url = '')
    {
        return $this->CurrentCountrySegment($url) ? true : false;
    }

    /**
     * returns the selected country code if there is one ...
     * as an uppercase code, e.g. NZ
     * @param string|null $url
     *
     * @return string|null
     */
    public function CurrentCountrySegment($url = '')
    {
        $url = $this->getCurrentURL($url);

        $parts = parse_url($url);
        if (isset($parts['path'])) {
            $path = trim($parts['path'], '/');
            $array = explode('/', $path);
            $potentialCountry = isset($array[0]) ? trim($array[0]) : '';
            if (strlen($potentialCountry) === 2) {
                $potentialCountry = strtoupper($potentialCountry);
                $check = EcommerceCountry::get()->filter(['Code' => $potentialCountry])->count();
                if ($check == 1) {
                    return $potentialCountry;
                }
            }
        }
    }

    /**
     * replaces a country code in a URL with another one
     *
     * @param  string $newCountryCode e.g. NZ / nz
     * @param  string $url
     *
     * @return string|null only returns a string if it is different from the original!
     */
    public function replaceCountryCodeInUrl($newCountryCode, $url = '')
    {
        $url = $this->getCurrentURL($url);

        $newCountryCode = strtolower($newCountryCode);
        $oldURL = $url;
        $parsedUrl = parse_url($url);
        if (isset($parsedUrl['path']) && isset($parsedUrl['host'])) {
            $path = $parsedUrl['path'];
            $path = trim($path, '/');
            $pathParts = explode('/', $path);
            $currentCountryCode = $this->CurrentCountrySegment($url);
<?php

function getDate($date)
{
    if ($date !== null) {
        return new DateTime($date);
    }

    return false;
}
            if ($currentCountryCode) {
''   == false // true
''   == null  // true
'ab' == false // false
'ab' == null  // false

// It is often better to use strict comparison
'' === false // false
'' === null  // false
                $pathParts[0] = $newCountryCode;
            } else {
                array_unshift($pathParts, $newCountryCode);
            }
            $parsedUrl['path'] = implode('/', $pathParts);
            $newURL =
                $parsedUrl['scheme'] .
                '://' .
                Controller::join_links(
                    $parsedUrl['host'],
                    $parsedUrl['path']
                );
            if (isset($parsedUrl['query'])) {
                $newURL = $newURL . '?' . $parsedUrl['query'];
            }
        }
        if ($oldURL !== $newURL) {
            return $newURL;
function myFunction($a) {
    switch ($a) {
        case 'foo':
            $x = 1;
            break;

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

    // $x is potentially undefined here.
    echo $x;
}
        }

        return '';
    }

    /**
     *
     * @param  string|null $url can be a relative one or nothing at all ...
     *
     * @return string      full URL currently being called.

     */
    public function getCurrentURL($url = '')
// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
    {
        if ($url) {
''   == false // true
''   == null  // true
'ab' == false // false
'ab' == null  // false

// It is often better to use strict comparison
'' === false // false
'' === null  // false
            return Director::absoluteURL($url);
        }
        $protocol = Director::is_https() ? 'https://' : 'http://';

        return  $protocol.$_SERVER['HTTP_HOST'].$_SERVER['REQUEST_URI'];
    }
}


1			<?php
2
3			/**
4			* Usage:
5			* $myAnswer =
6			* CountryPrice_Translation::get_country_url_provider()
7			* ->getSomething();
8			*
9			*/
10
11			class CountryURLProvider extends Object implements CountryURLProviderInterface
			0 ignored issues – show Coding Style Compatibility introduced 2017-03-30 04:17 UTC by Report Bug Copy Issue Report PSR1 recommends that each class must be in a namespace of at least one level to avoid collisions. You can fix this by adding a namespace to your class: namespace YourVendor; class YourClass { } When choosing a vendor namespace, try to pick something that is not too generic to avoid conflicts with other libraries. Loading history...
12			{
13
14
15			/**
16			* returns the selected country code if there is one ...
17			* as an uppercase code, e.g. NZ
18			* @param string\|null $url
19			*
20			* @return bool
21			*/
22			public function hasCountrySegment($url = '')
23			{
24			return $this->CurrentCountrySegment($url) ? true : false;
25			}
26
27			/**
28			* returns the selected country code if there is one ...
29			* as an uppercase code, e.g. NZ
30			* @param string\|null $url
31			*
32			* @return string\|null
33			*/
34			public function CurrentCountrySegment($url = '')
35			{
36			$url = $this->getCurrentURL($url);
37
38			$parts = parse_url($url);
39			if (isset($parts['path'])) {
40			$path = trim($parts['path'], '/');
41			$array = explode('/', $path);
42			$potentialCountry = isset($array[0]) ? trim($array[0]) : '';
43			if (strlen($potentialCountry) === 2) {
44			$potentialCountry = strtoupper($potentialCountry);
45			$check = EcommerceCountry::get()->filter(['Code' => $potentialCountry])->count();
46			if ($check == 1) {
47			return $potentialCountry;
48			}
49			}
50			}
51			}
52
53			/**
54			* replaces a country code in a URL with another one
55			*
56			* @param string $newCountryCode e.g. NZ / nz
57			* @param string $url
58			*
59			* @return string\|null only returns a string if it is different from the original!
60			*/
61			public function replaceCountryCodeInUrl($newCountryCode, $url = '')
62			{
63			$url = $this->getCurrentURL($url);
64
65			$newCountryCode = strtolower($newCountryCode);
66			$oldURL = $url;
67			$parsedUrl = parse_url($url);
68			if (isset($parsedUrl['path']) && isset($parsedUrl['host'])) {
69			$path = $parsedUrl['path'];
70			$path = trim($path, '/');
71			$pathParts = explode('/', $path);
72			$currentCountryCode = $this->CurrentCountrySegment($url);
			0 ignored issues – show Security Bug introduced 2018-02-14 08:36 UTC by Report Bug Copy Issue Report It seems like `$url` defined by `$this->getCurrentURL($url)` on line `63` can also be of type `false`; however, `CountryURLProvider::CurrentCountrySegment()` does only seem to accept `string\|null`, did you maybe forget to handle an error condition? This check looks for type mismatches where the missing type is `false`. This is usually indicative of an error condtion. Consider the follow example <?php function getDate($date) { if ($date !== null) { return new DateTime($date); } return false; } This function either returns a new `DateTime` object or false, if there was an error. This is a typical pattern in PHP programming to show that an error has occurred without raising an exception. The calling code should check for this returned `false` before passing on the value to another function or method that may not be able to handle a `false`. Loading history...
73			if ($currentCountryCode) {
			0 ignored issues – show Bug Best Practice introduced 2018-02-14 08:36 UTC by Report Bug Copy Issue Report The expression `$currentCountryCode` of type `string\|null` 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...
74			$pathParts[0] = $newCountryCode;
75			} else {
76			array_unshift($pathParts, $newCountryCode);
77			}
78			$parsedUrl['path'] = implode('/', $pathParts);
79			$newURL =
80			$parsedUrl['scheme'] .
81			'://' .
82			Controller::join_links(
83			$parsedUrl['host'],
84			$parsedUrl['path']
85			);
86			if (isset($parsedUrl['query'])) {
87			$newURL = $newURL . '?' . $parsedUrl['query'];
88			}
89			}
90			if ($oldURL !== $newURL) {
91			return $newURL;
			0 ignored issues – show Bug introduced 2018-02-14 08:36 UTC by Report Bug Copy Issue Report The variable `$newURL` does not seem to be defined for all execution paths leading up to this point. If you define a variable conditionally, it can happen that it is not defined for all execution paths. Let’s take a look at an example: function myFunction($a) { switch ($a) { case 'foo': $x = 1; break; case 'bar': $x = 2; break; } // $x is potentially undefined here. echo $x; } In the above example, the variable `$x` is defined if you pass “foo” or “bar” as argument for `$a`. However, since the `switch` statement has no default case statement, if you pass any other value, the variable `$x` would be undefined. Available Fixes Check for existence of the variable explicitly: function myFunction($a) { switch ($a) { case 'foo': $x = 1; break; case 'bar': $x = 2; break; } if (isset($x)) { // Make sure it's always set. echo $x; } } Define a default value for the variable: function myFunction($a) { $x = ''; // Set a default which gets overridden for certain paths. switch ($a) { case 'foo': $x = 1; break; case 'bar': $x = 2; break; } echo $x; } Add a value for the missing path: function myFunction($a) { switch ($a) { case 'foo': $x = 1; break; case 'bar': $x = 2; break; // We add support for the missing case. default: $x = ''; break; } echo $x; } Loading history...
92			}
93
94			return '';
95			}
96
97			/**
98			*
99			* @param string\|null $url can be a relative one or nothing at all ...
100			*
101			* @return string full URL currently being called.
			0 ignored issues – show Documentation introduced 2018-02-14 08:36 UTC by Report Bug Copy Issue Report Should the return type not be `false\|string`? This check compares the return type specified in the `@return` annotation of a function or method doc comment with the types returned by the function and raises an issue if they mismatch. Loading history...
102			*/
103			public function getCurrentURL($url = '')
			0 ignored issues – show Coding Style introduced 2018-02-14 08:36 UTC by Report Bug Copy Issue Report `getCurrentURL` uses the super-global variable `$_SERVER` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history...
104			{
105			if ($url) {
			0 ignored issues – show Bug Best Practice introduced 2018-02-14 08:36 UTC by Report Bug Copy Issue Report The expression `$url` of type `string\|null` 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...
106			return Director::absoluteURL($url);
107			}
108			$protocol = Director::is_https() ? 'https://' : 'http://';
109
110			return $protocol.$_SERVER['HTTP_HOST'].$_SERVER['REQUEST_URI'];
111			}
112			}
113

sunnysideup / silverstripe-ecommerce_countries

Push — master ( 244f37...eeee86 )

CountryURLProvider::replaceCountryCodeInUrl() B

Complexity

Size

Duplication

Importance

Available Fixes

Duplication Side-by-Side

Filter issues like