CountryPrices_ChangeCountryController::findNewURL() - Code Metrics - Inspection of "MINOR: less redirections ..." - sunnysideup/silverstripe-ecommerce_countries - Measure and Improve Code Quality continuously with Scrutinizer

Passed

Push — master ( 4ebc9d...305f3a )

by Nicolaas

created 2018-01-16 22:45 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
    );

    /**
     * only call this function if it is a NEW country we are dealing with!
     *
     * @param string $newCountryCode
     *
     */
    public static function set_new_country($newCountryCode)
    {
        $newCountryCode = strtoupper($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);

    }

    /**
     * link to change to a new country.
     *
     * @param string $newCountryCode
     *
     */
    public static function new_country_link($newCountryCode)

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

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

    /**
     *
     * @param  SS_HTTPRequest $request
     *
     * @return SS_HTTPResponse

     */
    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);
    }
}
    {
        //check for offsite redirects???

        $newCountryCode = strotoupper($request->param('ID'));
        self::set_new_country($newCountryCode);

        //redirect now
        $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			/**
36			* only call this function if it is a NEW country we are dealing with!
37			*
38			* @param string $newCountryCode
39			*
40			*/
41			public static function set_new_country($newCountryCode)
42			{
43			$newCountryCode = strtoupper($newCountryCode);
44
45			Session::set('MyCloudFlareCountry', $newCountryCode);
46			$o = Shoppingcart::current_order();
47			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...
48			//..
49			} else {
50			ShoppingCart::singleton()->clear();
51			}
52			CountryPrice_OrderDOD::localise_order($newCountryCode, true);
53
54			}
55
56			/**
57			* link to change to a new country.
58			*
59			* @param string $newCountryCode
60			*
61			*/
62			public static function new_country_link($newCountryCode)
			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...
63			{
64			$newCountryCode = strtoupper($newCountryCode);
65			$redirectsArray = Config::inst()->get('CountryPrices_ChangeCountryController', 'off_site_url_redirects');
66			if (isset($redirectsArray[$newCountryCode])) {
67			return $redirectsArray[$newCountryCode];
68			}
69
70			return Injector::inst()->get('CountryPrices_ChangeCountryController')->Link('changeto/'.strtolower($newCountryCode).'/');
71			}
72
73			/**
74			*
75			* @param SS_HTTPRequest $request
76			*
77			* @return SS_HTTPResponse
			0 ignored issues – show Documentation introduced 2018-01-16 22:48 UTC by Report Bug Copy Issue Report Should the return type not be `SS_HTTPResponse\|null`? 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...
78			*/
79			public function changeto($request)
			0 ignored issues – show Coding Style introduced 2018-01-16 22:48 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...
80			{
81			//check for offsite redirects???
			0 ignored issues – show Unused Code Comprehensibility introduced 2018-01-16 22:48 UTC by Report Bug Copy Issue Report `40%` of this comment could be valid code. Did you maybe forget this after debugging? Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it. The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production. This check looks for comments that seem to be mostly valid code and reports them. Loading history...
82			$newCountryCode = strotoupper($request->param('ID'));
83			self::set_new_country($newCountryCode);
84
85			//redirect now
86			$param = Config::inst()->get('CountryPrice_Translation', 'locale_get_parameter');
87			if(isset($_GET['force']) && $_GET['force']) {
88
89			return $this->redirect(self::$url_segment . '/changeto/' .$newCountryCode . '/'. '?force-back-home');
90			}
91			if(isset($_GET['force-back-home']) \|\| $_GET['force-back-home']) {
92
93			return $this->redirect('/');
94			}
95
96			return $this->redirect($this->findNewURL($param, $newCountryCode));
97			}
98
99			public function Link($action = null)
100			{
101			return Controller::join_links(Config::inst()->get('CountryPrices_ChangeCountryController', 'url_segment'), $action);
102			}
103
104			/**
105			* Remove a query string parameter from an URL.
106			*
107			* @param string $varname
108			* @param string $newCountryCode
109			*
110			* @return string
111			*/
112			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...
113			{
114
115			//COPIED FROM DIRECTOR::redirectBack()
116			// Don't cache the redirect back ever
117			HTTP::set_cache_age(0);
118
119			$url = null;
120
121			// In edge-cases, this will be called outside of a handleRequest() context; in that case,
122			// redirect to the homepage - don't break into the global state at this stage because we'll
123			// be calling from a test context or something else where the global state is inappropraite
124			if ($this->getRequest()) {
125			if ($this->getRequest()->requestVar('BackURL')) {
126			$url = $this->getRequest()->requestVar('BackURL');
127			} elseif ($this->getRequest()->isAjax() && $this->getRequest()->getHeader('X-Backurl')) {
128			$url = $this->getRequest()->getHeader('X-Backurl');
129			} elseif ($this->getRequest()->getHeader('Referer')) {
130			$url = $this->getRequest()->getHeader('Referer');
131			}
132			}
133
134			if (!$url) {
135			$url = Director::baseURL();
136			}
137			// absolute redirection URLs not located on this site may cause phishing
138			if (Director::is_site_url($url)) {
139			$url = Director::absoluteURL($url, true);
140			$parsedUrl = parse_url($url);
141			$query = array();
142
143			if (isset($parsedUrl['query'])) {
144			parse_str($parsedUrl['query'], $query);
145			if ($query[$varname] !== $newCountryCode) {
146			unset($query[$varname]);
147			}
148			}
149
150			$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...
151
152			$query = !empty($query) ? '?'. http_build_query($query) : '';
153
154			$hasCountrySegment = CountryPrice_Translation::get_country_url_provider()->hasCountrySegment($url);
155			if($hasCountrySegment){
156			$url = CountryPrice_Translation::get_country_url_provider()->replaceCountryCodeInUrl($newCountryCode, $url);
157			}
158			else {
159			$url = CountryPrice_Translation::get_country_url_provider()->addCountryCodeToUrl($newCountryCode, $url);
160			}
161			return $url.$query;
162			}
163			return '/';
164			}
165
166			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...
167			{
168			}
169			}
170

sunnysideup / silverstripe-ecommerce_countries

Push — master ( 4ebc9d...305f3a )

findNewURL() D

Complexity

Size

Duplication

Importance

How to fix Long Method Complexity

Long Method

Duplication Side-by-Side

Filter issues like