CountryPrice_Page_Controller_Extension::addCountryCodeToUrlIfRequired() - Code Metrics - Inspection of "MINOR: caching the hreflang links" - sunnysideup/silverstripe-ecommerce_countries - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( fb0bef...1ce86f )

by Nicolaas

created 2016-11-14 06:00 UTC

addCountryCodeToUrlIfRequired() B

↳ Parent: CountryPrice_Page_Controller_Extension

Complexity

Conditions	6
Paths	5

Size

Total Lines	30
Code Lines	17

Duplication

Lines	0
Ratio	0 %

Importance

Changes

Metric	Value
dl	0
loc	30
rs	8.439
c	0
b	0
f	0
cc	6
eloc	17
nc	5
nop	1

<?php


/**
 * www.mysite.com/mypage/?ecomlocale=AU
 * if there is a tranlsation page redirects to
 * URL with ?ecomlocale=AU
 *
 * if you go to a URL with ?ecomlocale=AU and the shop country does not match
 * the get param then you get redirected to that shop country.
 *
 *
 */

class CountryPrice_Page_Controller_Extension extends Extension
namespace YourVendor;

class YourClass { }
{

    /**
     * replaces `Title` and `Content` with translated content
     * where available.
     *
     * If the country code in the get parameter is not correct then
     * @return [type] [description]

     */
    public function onAfterInit()
// 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);
    }
}
    {
        $countryID = 0;
        $param = Config::inst()->get('CountryPrice_Translation', 'locale_get_parameter');
        $countryObject = CountryPrice_EcommerceCountry::get_real_country();
        if (isset($_GET[$param])) {
            $countryCode = preg_replace("/[^A-Z]+/", "", strtoupper(Convert::raw2sql($_GET[$param])));
            if ($countryObject->Code != $countryCode) {
                return $this->owner->redirect(
                    CountryPrices_ChangeCountryController::new_country_link($countryCode)
                );
            }
        }

        if ($countryObject) {
            $countryID = $countryObject->ID;
            $newURL = $this->addCountryCodeToUrlIfRequired($countryObject->Code);
            if($newURL) {
''   == false // true
''   == null  // true
'ab' == false // false
'ab' == null  // false

// It is often better to use strict comparison
'' === false // false
'' === null  // false
                $this->owner->redirect($newURL);
            }
        }

        $this->owner->dataRecord->loadTranslatedValues($countryID, null);
    }

    /**
     * returns the best fieldname for
     * @param string $fieldName [description]
     */
    public function CountryDistributorBestContentValue($fieldName)
    {
        $countryObject = CountryPrice_EcommerceCountry::get_real_country();

        //check country
        if (!empty($countryObject->$fieldName)) {
            return $countryObject->$fieldName;
        }

        //check distributor
        $distributor = Distributor::get_one_for_country($countryObject->Code);
        if (!empty($distributor->$fieldName)) {
            return $distributor->$fieldName;
        }
        //check EcomConfig
        $distributor = Distributor::get_one_for_country($countryObject->Code);
        if (!empty($distributor->$fieldName)) {
            return $distributor->$fieldName;
        }
    }

    /**
     * caching variable
     *
     * @var integer
     */
    private static $_redirection_count = 0;

    /**
     * returns a string for the new url if a locale parameter can be added
     *
     * @return string | null
     */
    private function addCountryCodeToUrlIfRequired($countryCode = '')
// 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 (isset($_POST) && count($_POST)) {
            return null;
        }
        $oldURL = "http://$_SERVER[HTTP_HOST]$_SERVER[REQUEST_URI]";

        $urlParts = parse_url($oldURL);
        parse_str($urlParts['query'], $params);

        $param = Config::inst()->get('CountryPrice_Translation', 'locale_get_parameter');
        $params[$param] = $countryCode;     // Overwrite if exists

        // Note that this will url_encode all values
        $urlParts['query'] = http_build_query($params);

        // If you have pecl_http
        if (function_exists('http_build_url')) {
            $newURL = http_build_url($urlParts);
        } else {
            $newURL =  $urlParts['scheme'] . '://' . $urlParts['host'] . $urlParts['path'] . '?' . $urlParts['query'];
        }

        if ($oldURL !== $newURL && self::$_redirection_count < 3) {
            self::$_redirection_count++;
            return $newURL;
        }

        return null;
    }


    /**
     *
     *
     * @return ArrayList
     */
    public function ChooseNewCountryList()
    {
        $countries = CountryPrice_EcommerceCountry::get_real_countries_list();
        $currentCode = '';
        if ($obj = CountryPrice_EcommerceCountry::get_real_country()) {
            $currentCode = $obj->Code;
        }
        $al = ArrayList::create();
        foreach ($countries as $country) {
            $isCurrentOne = $currentCode == $country->Code ? true : false;
            $currency = null;
$myVar = 'Value';
$higher = false;

if (rand(1, 6) > 3) {
    $higher = true;
} else {
    $higher = false;
}
            $currency = CountryPrice_EcommerceCurrency::get_currency_for_country($country->Code);
            $currencyCode = CountryPrice_EcommerceCurrency::get_currency_for_country($country->Code);
$myVar = 'Value';
$higher = false;

if (rand(1, 6) > 3) {
    $higher = true;
} else {
    $higher = false;
}
            $al->push(
                ArrayData::create(
                    array(
                        'Link' => CountryPrices_ChangeCountryController::new_country_link($country->Code),
                        'Title' => $country->Name,
                        'CountryCode' => $country->Code,
                        'LinkingMode' => ($isCurrentOne ? 'current' : 'link'),
                        'Currency' => $currency,
                        'CurrencyCode' => $currency
                    )
                )
            );
        }
        return $al;
    }

    /**
     * 
     * @return DataList
     */
    function AlternativeHrefLangLinksCachingKey()

    {
        return 'AlternativeHrefLangLinksCachingKey'.'-'.$this->owner->dataRecord->ID.'-'.strtotime($this->owner->dataRecord->LastEdited);
    }
}


1			<?php
2
3
4			/**
5			* www.mysite.com/mypage/?ecomlocale=AU
6			* if there is a tranlsation page redirects to
7			* URL with ?ecomlocale=AU
8			*
9			* if you go to a URL with ?ecomlocale=AU and the shop country does not match
10			* the get param then you get redirected to that shop country.
11			*
12			*
13			*/
14
15			class CountryPrice_Page_Controller_Extension extends Extension
			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...
16			{
17
18			/**
19			* replaces `Title` and `Content` with translated content
20			* where available.
21			*
22			* If the country code in the get parameter is not correct then
23			* @return [type] [description]
			0 ignored issues – show Documentation introduced 2016-10-31 22:45 UTC by Report Bug Copy Issue Report The doc-type `[type]` could not be parsed: Unknown type name "" at position 0. [(view supported doc-types) This check marks PHPDoc comments that could not be parsed by our parser. To see which comment annotations we can parse, please refer to our documentation on supported doc-types. Loading history...
24			*/
25			public function onAfterInit()
			0 ignored issues – show Coding Style introduced 2016-10-31 22:45 UTC by Report Bug Copy Issue Report `onAfterInit` 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...
26			{
27			$countryID = 0;
28			$param = Config::inst()->get('CountryPrice_Translation', 'locale_get_parameter');
29			$countryObject = CountryPrice_EcommerceCountry::get_real_country();
30			if (isset($_GET[$param])) {
31			$countryCode = preg_replace("/[^A-Z]+/", "", strtoupper(Convert::raw2sql($_GET[$param])));
32			if ($countryObject->Code != $countryCode) {
33			return $this->owner->redirect(
34			CountryPrices_ChangeCountryController::new_country_link($countryCode)
35			);
36			}
37			}
38
39			if ($countryObject) {
40			$countryID = $countryObject->ID;
41			$newURL = $this->addCountryCodeToUrlIfRequired($countryObject->Code);
42			if($newURL) {
			0 ignored issues – show Bug Best Practice introduced 2016-11-14 05:39 UTC by Report Bug Copy Issue Report The expression `$newURL` 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...
43			$this->owner->redirect($newURL);
44			}
45			}
46
47			$this->owner->dataRecord->loadTranslatedValues($countryID, null);
48			}
49
50			/**
51			* returns the best fieldname for
52			* @param string $fieldName [description]
53			*/
54			public function CountryDistributorBestContentValue($fieldName)
55			{
56			$countryObject = CountryPrice_EcommerceCountry::get_real_country();
57
58			//check country
59			if (!empty($countryObject->$fieldName)) {
60			return $countryObject->$fieldName;
61			}
62
63			//check distributor
64			$distributor = Distributor::get_one_for_country($countryObject->Code);
65			if (!empty($distributor->$fieldName)) {
66			return $distributor->$fieldName;
67			}
68			//check EcomConfig
69			$distributor = Distributor::get_one_for_country($countryObject->Code);
70			if (!empty($distributor->$fieldName)) {
71			return $distributor->$fieldName;
72			}
73			}
74
75			/**
76			* caching variable
77			*
78			* @var integer
79			*/
80			private static $_redirection_count = 0;
81
82			/**
83			* returns a string for the new url if a locale parameter can be added
84			*
85			* @return string \| null
86			*/
87			private function addCountryCodeToUrlIfRequired($countryCode = '')
			0 ignored issues – show Coding Style introduced 2016-10-31 22:45 UTC by Report Bug Copy Issue Report `addCountryCodeToUrlIfRequired` uses the super-global variable `$_POST` 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... Coding Style introduced 2016-10-31 22:45 UTC by Report Bug Copy Issue Report `addCountryCodeToUrlIfRequired` 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...
88			{
89			if (isset($_POST) && count($_POST)) {
90			return null;
91			}
92			$oldURL = "http://$_SERVER[HTTP_HOST]$_SERVER[REQUEST_URI]";
93
94			$urlParts = parse_url($oldURL);
95			parse_str($urlParts['query'], $params);
96
97			$param = Config::inst()->get('CountryPrice_Translation', 'locale_get_parameter');
98			$params[$param] = $countryCode; // Overwrite if exists
99
100			// Note that this will url_encode all values
101			$urlParts['query'] = http_build_query($params);
102
103			// If you have pecl_http
104			if (function_exists('http_build_url')) {
105			$newURL = http_build_url($urlParts);
106			} else {
107			$newURL = $urlParts['scheme'] . '://' . $urlParts['host'] . $urlParts['path'] . '?' . $urlParts['query'];
108			}
109
110			if ($oldURL !== $newURL && self::$_redirection_count < 3) {
111			self::$_redirection_count++;
112			return $newURL;
113			}
114
115			return null;
116			}
117
118
119			/**
120			*
121			*
122			* @return ArrayList
123			*/
124			public function ChooseNewCountryList()
125			{
126			$countries = CountryPrice_EcommerceCountry::get_real_countries_list();
127			$currentCode = '';
128			if ($obj = CountryPrice_EcommerceCountry::get_real_country()) {
129			$currentCode = $obj->Code;
130			}
131			$al = ArrayList::create();
132			foreach ($countries as $country) {
133			$isCurrentOne = $currentCode == $country->Code ? true : false;
134			$currency = null;
			0 ignored issues – show Unused Code introduced 2016-10-31 22:45 UTC by Report Bug Copy Issue Report `$currency` 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...
135			$currency = CountryPrice_EcommerceCurrency::get_currency_for_country($country->Code);
136			$currencyCode = CountryPrice_EcommerceCurrency::get_currency_for_country($country->Code);
			0 ignored issues – show Unused Code introduced 2016-10-31 22:45 UTC by Report Bug Copy Issue Report `$currencyCode` 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...
137			$al->push(
138			ArrayData::create(
139			array(
140			'Link' => CountryPrices_ChangeCountryController::new_country_link($country->Code),
141			'Title' => $country->Name,
142			'CountryCode' => $country->Code,
143			'LinkingMode' => ($isCurrentOne ? 'current' : 'link'),
144			'Currency' => $currency,
145			'CurrencyCode' => $currency
146			)
147			)
148			);
149			}
150			return $al;
151			}
152
153			/**
154			*
155			* @return DataList
156			*/
157			function AlternativeHrefLangLinksCachingKey()
			0 ignored issues – show Best Practice introduced 2016-11-14 06:03 UTC by Report Bug Copy Issue Report It is generally recommended to explicitly declare the visibility for methods. Adding explicit visibility (`private`, `protected`, or `public`) is generally recommend to communicate to other developers how, and from where this method is intended to be used. Loading history...
158			{
159			return 'AlternativeHrefLangLinksCachingKey'.'-'.$this->owner->dataRecord->ID.'-'.strtotime($this->owner->dataRecord->LastEdited);
160			}
161			}
162

sunnysideup / silverstripe-ecommerce_countries

Push — master ( fb0bef...1ce86f )

addCountryCodeToUrlIfRequired() B

Complexity

Size

Duplication

Importance

Duplication Side-by-Side

Filter issues like