CountryPrices_ChangeCountryController::findNewURL() - Code Metrics - Inspection of "MINOR: allow country change to be forced" - sunnysideup/silverstripe-ecommerce_countries - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( 1717c1...fbb6b4 )

by Nicolaas

created 2017-12-04 04:57 UTC

findNewURL() D

↳ Parent: CountryPrices_ChangeCountryController

Complexity

Conditions	13
Paths	250

Size

Total Lines	53
Code Lines	29

Duplication

Lines	0
Ratio	0 %

Importance

Changes

Metric	Value
dl	0
loc	53
rs	4.6566
c	0
b	0
f	0
cc	13
eloc	29
nc	250
nop	2

How to fix Long Method Complexity

<?php

/**
 *
 * Common use:
 * ```php
 *    CountryPrices_ChangeCountryController::changeto('XX');
 *    CountryPrices_ChangeCountryController::new_country_link('XX');
 * ```
 *
 */

class CountryPrices_ChangeCountryController extends ContentController
namespace YourVendor;

class YourClass { }
{

    /**
     * make sure to match route...
     * @var string
     */
    private static $url_segment = 'shoppingcart-countries';

    /**
     * needs to be saved like this:
     * ZA => 'myshop.co.za'
     *
     * @var array
     */
    private static $off_site_url_redirects = array();


    private static $allowed_actions = array(

        "changeto" => true,
        "confirmredirection" => true
    );

    public static function new_country_link($countryCode)

    {
        $redirectsArray = Config::inst()->get('CountryPrices_ChangeCountryController', 'off_site_url_redirects');
        if (isset($redirectsArray[$countryCode])) {
            return $redirectsArray[$countryCode];
        }

        return Injector::inst()->get('CountryPrices_ChangeCountryController')->Link('changeto/'.$countryCode.'/');
    }

    public function changeto($request)
// 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);
    }
}
    {
        $redirectsArray = Config::inst()->get('CountryPrices_ChangeCountryController', 'off_site_url_redirects');
        $newCountryCode = substr(strtoupper($request->param('ID')), 0, 2);
        if (isset($redirectsArray[$newCountryCode])) {
            return $this->redirect($redirectsArray[$newCountryCode]);
        }
        Session::set('MyCloudFlareCountry', $newCountryCode);
        $o = Shoppingcart::current_order();
        if ($o && ($o->getCountry() == $newCountryCode)) {
if (rand(1, 6) > 3) {
//print "Check failed";
} else {
    print "Check succeeded";
}
            //..
        } else {
            ShoppingCart::singleton()->clear();
        }
        CountryPrice_OrderDOD::localise_order($newCountryCode, true);
        $param = Config::inst()->get('CountryPrice_Translation', 'locale_get_parameter');
        if(isset($_GET['force']) && $_GET['force']) {
            return $this->redirect(self::$url_segment . '/changeto/' .$newCountryCode . '/'. '?force-back-home');
        }
        if(isset($_GET['force-back-home']) || $_GET['force-back-home']) {
            return $this->redirect('/');
        }
        return $this->redirect($this->findNewURL($param, $newCountryCode));
    }

    public function Link($action = null)
    {
        return Controller::join_links(Config::inst()->get('CountryPrices_ChangeCountryController', 'url_segment'), $action);
    }

    /**
     * Remove a query string parameter from an URL.
     *
     * @param string $varname
     * @param string $newCountryCode
     *
     * @return string
     */
    public function findNewURL($varname = 'ecomlocale', $newCountryCode)
// $a must always be passed; it's default value is never used.
function someFunction($a = 5, $b) { }
    {

        //COPIED FROM DIRECTOR::redirectBack()
        // Don't cache the redirect back ever
        HTTP::set_cache_age(0);

        $url = null;

        // In edge-cases, this will be called outside of a handleRequest() context; in that case,
        // redirect to the homepage - don't break into the global state at this stage because we'll
        // be calling from a test context or something else where the global state is inappropraite
        if ($this->getRequest()) {
            if ($this->getRequest()->requestVar('BackURL')) {
                $url = $this->getRequest()->requestVar('BackURL');
            } elseif ($this->getRequest()->isAjax() && $this->getRequest()->getHeader('X-Backurl')) {
                $url = $this->getRequest()->getHeader('X-Backurl');
            } elseif ($this->getRequest()->getHeader('Referer')) {
                $url = $this->getRequest()->getHeader('Referer');
            }
        }

        if (!$url) {
            $url = Director::baseURL();
        }
        // absolute redirection URLs not located on this site may cause phishing
        if (Director::is_site_url($url)) {
            $url = Director::absoluteURL($url, true);
            $parsedUrl = parse_url($url);
            $query = array();

            if (isset($parsedUrl['query'])) {
                parse_str($parsedUrl['query'], $query);
                if ($query[$varname] !== $newCountryCode) {
                    unset($query[$varname]);
                }
            }

            $path = isset($parsedUrl['path']) ? $parsedUrl['path'] : '';
$myVar = 'Value';
$higher = false;

if (rand(1, 6) > 3) {
    $higher = true;
} else {
    $higher = false;
}

            $query = !empty($query) ? '?'. http_build_query($query) : '';

            $hasCountrySegment = CountryPrice_Translation::get_country_url_provider()->hasCountrySegment($url);
            if($hasCountrySegment){
                $url = CountryPrice_Translation::get_country_url_provider()->replaceCountryCodeInUrl($newCountryCode, $url);
            }
            else {
                $url = CountryPrice_Translation::get_country_url_provider()->addCountryCodeToUrl($newCountryCode, $url);
            }
            return $url.$query;
        }
        return '/';
    }

    public function confirmredirection($request)

    {
    }
}


1			<?php
2
3			/**
4			*
5			* Common use:
6			* ```php
7			* CountryPrices_ChangeCountryController::changeto('XX');
8			* CountryPrices_ChangeCountryController::new_country_link('XX');
9			* ```
10			*
11			*/
12
13			class CountryPrices_ChangeCountryController extends ContentController
			0 ignored issues – show Coding Style Compatibility introduced 2016-10-31 22:45 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...
14			{
15
16			/**
17			* make sure to match route...
18			* @var string
19			*/
20			private static $url_segment = 'shoppingcart-countries';
21
22			/**
23			* needs to be saved like this:
24			* ZA => 'myshop.co.za'
25			*
26			* @var array
27			*/
28			private static $off_site_url_redirects = array();
			0 ignored issues – show Unused Code introduced 2016-10-31 22:45 UTC by Report Bug Copy Issue Report The property `$off_site_url_redirects` 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...
29
30			private static $allowed_actions = array(
			0 ignored issues – show Comprehensibility introduced 2016-10-31 22:45 UTC by Report Bug Copy Issue Report Consider using a different property name as you override a private property of the parent class. Loading history... Unused Code introduced 2016-10-31 22:45 UTC by Report Bug Copy Issue Report The property `$allowed_actions` 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...
31			"changeto" => true,
32			"confirmredirection" => true
33			);
34
35			public static function new_country_link($countryCode)
			0 ignored issues – show Documentation introduced 2016-10-31 22:45 UTC by Report Bug Copy Issue Report The return type could not be reliably inferred; please add a `@return` annotation. Our type inference engine in quite powerful, but sometimes the code does not provide enough clues to go by. In these cases we request you to add a `@return` annotation as described here. Loading history...
36			{
37			$redirectsArray = Config::inst()->get('CountryPrices_ChangeCountryController', 'off_site_url_redirects');
38			if (isset($redirectsArray[$countryCode])) {
39			return $redirectsArray[$countryCode];
40			}
41
42			return Injector::inst()->get('CountryPrices_ChangeCountryController')->Link('changeto/'.$countryCode.'/');
43			}
44
45			public function changeto($request)
			0 ignored issues – show Coding Style introduced 2017-12-04 05:01 UTC by Report Bug Copy Issue Report `changeto` 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...
46			{
47			$redirectsArray = Config::inst()->get('CountryPrices_ChangeCountryController', 'off_site_url_redirects');
48			$newCountryCode = substr(strtoupper($request->param('ID')), 0, 2);
49			if (isset($redirectsArray[$newCountryCode])) {
50			return $this->redirect($redirectsArray[$newCountryCode]);
51			}
52			Session::set('MyCloudFlareCountry', $newCountryCode);
53			$o = Shoppingcart::current_order();
54			if ($o && ($o->getCountry() == $newCountryCode)) {
			0 ignored issues – show Unused Code introduced 2016-10-31 22:45 UTC by Report Bug Copy Issue Report This `if` statement is empty and can be removed. This check looks for the bodies of `if` statements that have no statements or where all statements have been commented out. This may be the result of changes for debugging or the code may simply be obsolete. These `if` bodies can be removed. If you have an empty if but statements in the `else` branch, consider inverting the condition. if (rand(1, 6) > 3) { //print "Check failed"; } else { print "Check succeeded"; } could be turned into if (rand(1, 6) <= 3) { print "Check succeeded"; } This is much more concise to read. Loading history...
55			//..
56			} else {
57			ShoppingCart::singleton()->clear();
58			}
59			CountryPrice_OrderDOD::localise_order($newCountryCode, true);
60			$param = Config::inst()->get('CountryPrice_Translation', 'locale_get_parameter');
61			if(isset($_GET['force']) && $_GET['force']) {
62			return $this->redirect(self::$url_segment . '/changeto/' .$newCountryCode . '/'. '?force-back-home');
63			}
64			if(isset($_GET['force-back-home']) \|\| $_GET['force-back-home']) {
65			return $this->redirect('/');
66			}
67			return $this->redirect($this->findNewURL($param, $newCountryCode));
68			}
69
70			public function Link($action = null)
71			{
72			return Controller::join_links(Config::inst()->get('CountryPrices_ChangeCountryController', 'url_segment'), $action);
73			}
74
75			/**
76			* Remove a query string parameter from an URL.
77			*
78			* @param string $varname
79			* @param string $newCountryCode
80			*
81			* @return string
82			*/
83			public function findNewURL($varname = 'ecomlocale', $newCountryCode)
			0 ignored issues – show Coding Style introduced 2017-10-18 01:15 UTC by Report Bug Copy Issue Report Parameters which have default values should be placed at the end. If you place a parameter with a default value before a parameter with a default value, the default value of the first parameter will never be used as it will always need to be passed anyway: // $a must always be passed; it's default value is never used. function someFunction($a = 5, $b) { } Loading history...
84			{
85
86			//COPIED FROM DIRECTOR::redirectBack()
87			// Don't cache the redirect back ever
88			HTTP::set_cache_age(0);
89
90			$url = null;
91
92			// In edge-cases, this will be called outside of a handleRequest() context; in that case,
93			// redirect to the homepage - don't break into the global state at this stage because we'll
94			// be calling from a test context or something else where the global state is inappropraite
95			if ($this->getRequest()) {
96			if ($this->getRequest()->requestVar('BackURL')) {
97			$url = $this->getRequest()->requestVar('BackURL');
98			} elseif ($this->getRequest()->isAjax() && $this->getRequest()->getHeader('X-Backurl')) {
99			$url = $this->getRequest()->getHeader('X-Backurl');
100			} elseif ($this->getRequest()->getHeader('Referer')) {
101			$url = $this->getRequest()->getHeader('Referer');
102			}
103			}
104
105			if (!$url) {
106			$url = Director::baseURL();
107			}
108			// absolute redirection URLs not located on this site may cause phishing
109			if (Director::is_site_url($url)) {
110			$url = Director::absoluteURL($url, true);
111			$parsedUrl = parse_url($url);
112			$query = array();
113
114			if (isset($parsedUrl['query'])) {
115			parse_str($parsedUrl['query'], $query);
116			if ($query[$varname] !== $newCountryCode) {
117			unset($query[$varname]);
118			}
119			}
120
121			$path = isset($parsedUrl['path']) ? $parsedUrl['path'] : '';
			0 ignored issues – show Unused Code introduced 2017-03-30 04:17 UTC by Report Bug Copy Issue Report `$path` is not used, you could remove the assignment. This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently. $myVar = 'Value'; $higher = false; if (rand(1, 6) > 3) { $higher = true; } else { $higher = false; } Both the `$myVar` assignment in line 1 and the `$higher` assignment in line 2 are dead. The first because `$myVar` is never used and the second because `$higher` is always overwritten for every possible time line. Loading history...
122
123			$query = !empty($query) ? '?'. http_build_query($query) : '';
124
125			$hasCountrySegment = CountryPrice_Translation::get_country_url_provider()->hasCountrySegment($url);
126			if($hasCountrySegment){
127			$url = CountryPrice_Translation::get_country_url_provider()->replaceCountryCodeInUrl($newCountryCode, $url);
128			}
129			else {
130			$url = CountryPrice_Translation::get_country_url_provider()->addCountryCodeToUrl($newCountryCode, $url);
131			}
132			return $url.$query;
133			}
134			return '/';
135			}
136
137			public function confirmredirection($request)
			0 ignored issues – show Unused Code introduced 2017-10-18 01:15 UTC by Report Bug Copy Issue Report The parameter `$request` is not used and could be removed. This check looks from parameters that have been defined for a function or method, but which are not used in the method body. Loading history...
138			{
139			}
140			}
141

sunnysideup / silverstripe-ecommerce_countries

Push — master ( 1717c1...fbb6b4 )

findNewURL() D

Complexity

Size

Duplication

Importance

How to fix Long Method Complexity

Long Method

Duplication Side-by-Side

Filter issues like