CountryURLProvider::getCurrentURL() - Code Metrics - Inspection of "PATCH: url redirection fix" - sunnysideup/silverstripe-ecommerce_countries - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( 721e90...cb7420 )

by Nicolaas

created 2018-05-14 01:48 UTC

CountryURLProvider::getCurrentURL() A

↳ Parent: CountryURLProvider

Complexity

Conditions	4
Paths	6

Size

Total Lines	15
Code Lines	10

Duplication

Lines	0
Ratio	0 %

Importance

Changes

Metric	Value
dl	0
loc	15
rs	9.2
c	0
b	0
f	0
cc	4
eloc	10
nc	6
nop	1

<?php

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

class CountryURLProvider extends Object implements CountryURLProviderInterface
namespace YourVendor;

class YourClass { }
{
    /**
     * @var string
     */
    private static $locale_get_parameter = 'ecomlocale';


    /**
     * 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 onCurrentCountrySegmente ...
     * as an uppercase code, e.g. NZ
     * @param string|null $url
     * @param bool $includeGetVariable
     *
     * @return string|null
     */
    public function CurrentCountrySegment($url = '', $includeGetVariable = true)
// 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);
    }
}
    {
        $potentialCountry = '';
        if ($includeGetVariable) {
            $getVar = Config::inst()->get('CountryURLProvider', 'locale_get_parameter');
            if(isset($_GET[$getVar])) {
                $potentialCountry = $_GET[$getVar];
            }
        }
        if (strlen($potentialCountry) !== 2) {
            $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) {
                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);
        $oldURL = $url;
        debug::log($url);

        $newCountryCode = strtolower($newCountryCode);
        $parsedUrl = parse_url($url);
        if (isset($parsedUrl['path']) && isset($parsedUrl['host'])) {
            $path = $parsedUrl['path'];
            $path = trim($path, '/');
            $pathParts = explode('/', $path);

            $currentCountryCode = $this->CurrentCountrySegment($url, false);
<?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 (trim($oldURL, '/') !== trim($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
            $url = Director::absoluteURL($url);
        } else {
            $protocol = Director::is_https() ? 'https://' : 'http://';

            $url = $protocol.$_SERVER['HTTP_HOST'].$_SERVER['REQUEST_URI'];
        }
        if (Director::is_site_url($url)) {
<?php

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

    return false;
}
            return $url;
        } else {
            return Director::absoluteURL('/');
        }
    }
}


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			* @var string
15			*/
16			private static $locale_get_parameter = 'ecomlocale';
			0 ignored issues – show Unused Code introduced 2016-11-14 05:39 UTC by Report Bug Copy Issue Report The property `$locale_get_parameter` is not used and could be removed. This check marks private properties in classes that are never used. Those properties can be removed. Loading history...
17
18			/**
19			* returns the selected country code if there is one ...
20			* as an uppercase code, e.g. NZ
21			* @param string\|null $url
22			*
23			* @return bool
24			*/
25			public function hasCountrySegment($url = '')
26			{
27			return $this->CurrentCountrySegment($url) ? true : false;
28			}
29
30			/**
31			* returns the selected country code if there is onCurrentCountrySegmente ...
32			* as an uppercase code, e.g. NZ
33			* @param string\|null $url
34			* @param bool $includeGetVariable
35			*
36			* @return string\|null
37			*/
38			public function CurrentCountrySegment($url = '', $includeGetVariable = true)
			0 ignored issues – show Coding Style introduced 2018-05-14 01:55 UTC by Report Bug Copy Issue Report `CurrentCountrySegment` uses the super-global variable `$_GET` 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...
39			{
40			$potentialCountry = '';
41			if ($includeGetVariable) {
42			$getVar = Config::inst()->get('CountryURLProvider', 'locale_get_parameter');
43			if(isset($_GET[$getVar])) {
44			$potentialCountry = $_GET[$getVar];
45			}
46			}
47			if (strlen($potentialCountry) !== 2) {
48			$url = $this->getCurrentURL($url);
49			$parts = parse_url($url);
50			if (isset($parts['path'])) {
51			$path = trim($parts['path'], '/');
52			$array = explode('/', $path);
53			$potentialCountry = isset($array[0]) ? trim($array[0]) : '';
54			}
55			}
56			if (strlen($potentialCountry) === 2) {
57			$potentialCountry = strtoupper($potentialCountry);
58			$check = EcommerceCountry::get()->filter(['Code' => $potentialCountry])->count();
59			if ($check) {
60			return $potentialCountry;
61			}
62			}
63			}
64
65
66			/**
67			* replaces a country code in a URL with another one
68			*
69			* @param string $newCountryCode e.g. NZ / nz
70			* @param string $url
71			*
72			* @return string\|null only returns a string if it is different from the original!
73			*/
74			public function replaceCountryCodeInUrl($newCountryCode, $url = '')
75			{
76			$url = $this->getCurrentURL($url);
77			$oldURL = $url;
78			debug::log($url);
79
80			$newCountryCode = strtolower($newCountryCode);
81			$parsedUrl = parse_url($url);
82			if (isset($parsedUrl['path']) && isset($parsedUrl['host'])) {
83			$path = $parsedUrl['path'];
84			$path = trim($path, '/');
85			$pathParts = explode('/', $path);
86
87			$currentCountryCode = $this->CurrentCountrySegment($url, false);
			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 `76` 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...
88			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...
89			$pathParts[0] = $newCountryCode;
90			} else {
91			array_unshift($pathParts, $newCountryCode);
92			}
93			$parsedUrl['path'] = implode('/', $pathParts);
94			$newURL =
95			$parsedUrl['scheme'] .
96			'://' .
97			Controller::join_links(
98			$parsedUrl['host'],
99			$parsedUrl['path']
100			);
101			if (isset($parsedUrl['query'])) {
102			$newURL = $newURL . '?' . $parsedUrl['query'];
103			}
104			}
105			if (trim($oldURL, '/') !== trim($newURL, '/')) {
106			return $newURL;
			0 ignored issues – show Bug introduced 2018-05-14 01:55 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...
107			}
108
109			return '';
110			}
111
112			/**
113			*
114			* @param string\|null $url can be a relative one or nothing at all ...
115			*
116			* @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...
117			*/
118			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...
119			{
120			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...
121			$url = Director::absoluteURL($url);
122			} else {
123			$protocol = Director::is_https() ? 'https://' : 'http://';
124
125			$url = $protocol.$_SERVER['HTTP_HOST'].$_SERVER['REQUEST_URI'];
126			}
127			if (Director::is_site_url($url)) {
			0 ignored issues – show Security Bug introduced 2018-05-09 03:51 UTC by Report Bug Copy Issue Report It seems like `$url` defined by `\Director::absoluteURL($url)` on line `121` can also be of type `false`; however, `Director::is_site_url()` does only seem to accept `string`, 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...
128			return $url;
129			} else {
130			return Director::absoluteURL('/');
131			}
132			}
133			}
134

sunnysideup / silverstripe-ecommerce_countries

Push — master ( 721e90...cb7420 )

CountryURLProvider::getCurrentURL() A

Complexity

Size

Duplication

Importance

Available Fixes

Duplication Side-by-Side

Filter issues like