Users_Account_Controller - Code Metrics - Inspection of "Minor code tweaks" - i-lateral/silverstripe-users - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — 1 ( 1e411c...a166aa )

by Morven

created 2018-02-21 15:56 UTC

Users_Account_Controller A

↳ Parent: Project

Complexity

Total Complexity

Size/Duplication

Total Lines	299
Duplicated Lines	11.04 %

Coupling/Cohesion

Components	2
Dependencies	14

Importance

Changes

Metric	Value
wmc	22
lcom	2
cbo	14
dl	33
loc	299
rs	10
c	0
b	0
f	0

14 Methods

Rating	Name	Duplication	Size	Complexity
A	getMember()	0	4	1
A	setMember()	0	5	1
A	RequireVerification()	0	8	3
A	init()	0	12	2
A	Link()	0	7	1
A	AbsoluteLink()	0	4	1
A	RelativeLink()	0	6	1
A	index()	16	16	1
A	edit()	17	17	1
B	changepassword()	0	34	3
A	EditAccountForm()	0	8	1
A	ChangePasswordForm()	0	23	1
B	getAccountMenu()	0	31	4
A	providePermissions()	0	17	1

How to fix Duplicated Code

<?php

/**
 * Controller that is used to allow users to manage their accounts via
 * the front end of the site.
 *
 */
class Users_Account_Controller extends Controller implements PermissionProvider
namespace YourVendor;

class YourClass { }
{

    /**
     * URL That you can access this from
     *
     * @config
     */
    private static $url_segment = "users/account";


    /**
     * Allowed sub-URL's on this controller
     * 
     * @var array
     * @config
     */
    private static $allowed_actions = array(

        "edit",
        "changepassword",
        "EditAccountForm",
        "ChangePasswordForm",
    );

    /**
     * User account associated with this controller
     *
     * @var Member
     */
    protected $member;

    /**
     * Getter for member
     *
     * @return Member
     */
    public function getMember()
    {
        return $this->member;
    }

    /**
     * Setter for member
     *
     * @param Member $member
     * @return self
     */
    public function setMember(Member $member)
    {
        $this->member = $member;
        return $this;
    }

    /**
     * Determine if current user requires verification (based on their
     * account and Users verification setting).
     *
     * @return boolean
     */
    public function RequireVerification()
    {
        if (!$this->member->isVerified() && Users::config()->require_verification) {

            return true;
        } else {
            return false;
        }
    }

    /**
     * Perorm setup when this controller is initialised
     *
     * @return void
     */
    public function init()
    {
        parent::init();

        // Check we are logged in as a user who can access front end management
        if (!Permission::check("USERS_MANAGE_ACCOUNT")) {
            Security::permissionFailure();
        }

        // Set our member object
        $this->member = Member::currentUser();
class Id
{
    public $id;

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

}

class Account
{
    /** @var  Id $id */
    public $id;
}

$account_id = false;

if (starsAreRight()) {
    $account_id = new Id(42);
}

$account = new Account();
if ($account instanceof Id)
{
    $account->id = $account_id;
}
    }

    /**
     * Get the link to this controller
     * 
     * @param string $action

     * @return string
     */
    public function Link($action = null)
    {
        return Controller::join_links(
            $this->config()->url_segment,
            $action
        );
    }

    /**
     * Get an absolute link to this controller
     *
     * @param string $action

     * @return string

     */
    public function AbsoluteLink($action = null)
    {
        return Director::absoluteURL($this->Link($action));
    }

    /**
     * Get a relative (to the root url of the site) link to this
     * controller
     *
     * @param string $action

     * @return string
     */
    public function RelativeLink($action = null)
    {
        return Controller::join_links(
            $this->Link($action)
        );
    }

    /**
     * Display the currently outstanding orders for the current user
     *
     */
    public function index()

    {
        $member = Member::currentUser();
$myVar = 'Value';
$higher = false;

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

        $this->customise(array(
            "ClassName" => "AccountPage"
        ));

        $this->extend("onBeforeIndex");

        return $this->renderWith(array(
            "UserAccount",
            "UserAccount",
            "Page"
        ));
    }

    public function edit()

    {
        $member = Member::currentUser();

        $this->customise(array(
            "ClassName" => "AccountPage",
            "Form"  => $this->EditAccountForm()->loadDataFrom($member)
/** @return stdClass|null */
function mayReturnNull() { }

function doesNotAcceptNull(stdClass $x) { }

// With potential error.
function withoutCheck() {
    $x = mayReturnNull();
    doesNotAcceptNull($x); // Potential error here.
}

// Safe - Alternative 1
function withCheck1() {
    $x = mayReturnNull();
    if ( ! $x instanceof stdClass) {
        throw new \LogicException('$x must be defined.');
    }
    doesNotAcceptNull($x);
}

// Safe - Alternative 2
function withCheck2() {
    $x = mayReturnNull();
    if ($x instanceof stdClass) {
        doesNotAcceptNull($x);
    }
}
        ));

        $this->extend("onBeforeEdit");

        return $this->renderWith(array(
            "UserAccount_edit",
            "UserAccount",
            "Page"
        ));
    }

    public function changepassword()
    {
        // Set the back URL for this form
        $back_url = Controller::join_links(
            $this->Link("changepassword"),
            "?s=1"
        );
        
        Session::set("BackURL", $back_url);
        
        $form = $this->ChangePasswordForm();
        
        // Is password changed, set a session message.
        $password_set = $this->request->getVar("s");
        if($password_set && $password_set == 1) {
            $form->sessionMessage(
                _t("Users.PasswordChangedSuccessfully","Password Changed Successfully"),
                "good"
            );
        }

        $this->customise(array(
            "ClassName" => "AccountPage",
            "Form"  => $form
        ));

        $this->extend("onBeforeChangePassword");

        return $this->renderWith(array(
            "UserAccount_changepassword",
            "UserAccount",
            "Page"
        ));
    }

    /**
     * Factory for generating a profile form. The form can be expanded using an
     * extension class and calling the updateEditProfileForm method.
     *
     * @return Form
     */
    public function EditAccountForm()
    {
        $form = Users_EditAccountForm::create($this, "EditAccountForm");

        $this->extend("updateEditAccountForm", $form);

        return $form;
    }

    /**
     * Factory for generating a change password form. The form can be expanded
     * using an extension class and calling the updateChangePasswordForm method.
     *
     * @return Form
     */
    public function ChangePasswordForm()
    {
        $form = ChangePasswordForm::create($this, "ChangePasswordForm");

        $form
            ->Actions()
            ->find("name", "action_doChangePassword")
            ->addExtraClass("btn")
            ->addExtraClass("btn-green");

        $cancel_btn = LiteralField::create(
            "CancelLink",
            '<a href="' . $this->Link() . '" class="btn btn-red">'. _t("Users.CANCEL", "Cancel") .'</a>'
        );

        $form
            ->Actions()
            ->insertBefore($cancel_btn, "action_doChangePassword");
function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);

        $this->extend("updateChangePasswordForm", $form);

        return $form;
    }

    /**
     * Return a list of nav items for managing a users profile. You can add new
     * items to this menu using the "updateAccountMenu" extension
     *
     * @return ArrayList
     */
    public function getAccountMenu()
    {
        $menu = new ArrayList();
        
        $curr_action = $this->request->param("Action");

        $menu->add(new ArrayData(array(
            "ID"    => 0,
            "Title" => _t('Users.PROFILESUMMARY', "Profile Summary"),
            "Link"  => $this->Link(),
            "LinkingMode" => (!$curr_action) ? "current" : "link"
        )));

        $menu->add(new ArrayData(array(
            "ID"    => 10,
            "Title" => _t('Users.EDITDETAILS', "Edit account details"),
            "Link"  => $this->Link("edit"),
            "LinkingMode" => ($curr_action == "edit") ? "current" : "link"
        )));

        $menu->add(new ArrayData(array(
            "ID"    => 30,
            "Title" => _t('Users.CHANGEPASSWORD', "Change password"),
            "Link"  => $this->Link("changepassword"),
            "LinkingMode" => ($curr_action == "changepassword") ? "current" : "link"
        )));

        $this->extend("updateAccountMenu", $menu);

        return $menu->sort("ID", "ASC");
    }

    public function providePermissions()
    {
        return array(
            "USERS_MANAGE_ACCOUNT" => array(
                'name' => 'Manage user account',
                'help' => 'Allow user to manage their account details',
                'category' => 'Frontend Users',
                'sort' => 100
            ),
            "USERS_VERIFIED" => array(
                'name' => 'Verified user',
                'help' => 'Users have verified their account',
                'category' => 'Frontend Users',
                'sort' => 100
            ),
        );
    }
}


1		<?php
2
3		/**
4		* Controller that is used to allow users to manage their accounts via
5		* the front end of the site.
6		*
7		*/
8		class Users_Account_Controller extends Controller implements PermissionProvider
		0 ignored issues – show Coding Style Compatibility introduced 2018-02-21 15:44 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...
9		{
10
11		/**
12		* URL That you can access this from
13		*
14		* @config
15		*/
16		private static $url_segment = "users/account";
		0 ignored issues – show Unused Code introduced 2018-02-21 15:44 UTC by Report Bug Copy Issue Report The property `$url_segment` 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		* Allowed sub-URL's on this controller
20		*
21		* @var array
22		* @config
23		*/
24		private static $allowed_actions = array(
		0 ignored issues – show Comprehensibility introduced 2018-02-21 15:44 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 2018-02-21 15:44 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...
25		"edit",
26		"changepassword",
27		"EditAccountForm",
28		"ChangePasswordForm",
29		);
30
31		/**
32		* User account associated with this controller
33		*
34		* @var Member
35		*/
36		protected $member;
37
38		/**
39		* Getter for member
40		*
41		* @return Member
42		*/
43		public function getMember()
44		{
45		return $this->member;
46		}
47
48		/**
49		* Setter for member
50		*
51		* @param Member $member
52		* @return self
53		*/
54		public function setMember(Member $member)
55		{
56		$this->member = $member;
57		return $this;
58		}
59
60		/**
61		* Determine if current user requires verification (based on their
62		* account and Users verification setting).
63		*
64		* @return boolean
65		*/
66		public function RequireVerification()
67		{
68		if (!$this->member->isVerified() && Users::config()->require_verification) {
		0 ignored issues – show Coding Style introduced 2018-02-21 15:44 UTC by Report Bug Copy Issue Report The `if`-`else` statement can be simplified to `return !$this->member->i...->require_verification;`. Loading history...
69		return true;
70		} else {
71		return false;
72		}
73		}
74
75		/**
76		* Perorm setup when this controller is initialised
77		*
78		* @return void
79		*/
80		public function init()
81		{
82		parent::init();
83
84		// Check we are logged in as a user who can access front end management
85		if (!Permission::check("USERS_MANAGE_ACCOUNT")) {
86		Security::permissionFailure();
87		}
88
89		// Set our member object
90		$this->member = Member::currentUser();
		0 ignored issues – show Documentation Bug introduced 2018-02-21 15:59 UTC by Report Bug Copy Issue Report It seems like `\Member::currentUser()` can also be of type `object<DataObject>`. However, the property `$member` is declared as type `object<Member>`. Maybe add an additional type check? Our type inference engine has found a suspicous assignment of a value to a property. This check raises an issue when a value that can be of a mixed type is assigned to a property that is type hinted more strictly. For example, imagine you have a variable `$accountId` that can either hold an Id object or false (if there is no account id yet). Your code now assigns that value to the `id` property of an instance of the `Account` class. This class holds a proper account, so the id value must no longer be false. Either this assignment is in error or a type check should be added for that assignment. class Id { public $id; public function __construct($id) { $this->id = $id; } } class Account { /** @var Id $id */ public $id; } $account_id = false; if (starsAreRight()) { $account_id = new Id(42); } $account = new Account(); if ($account instanceof Id) { $account->id = $account_id; } Loading history...
91		}
92
93		/**
94		* Get the link to this controller
95		*
96		* @param string $action
		0 ignored issues – show Documentation introduced 2018-02-21 15:59 UTC by Report Bug Copy Issue Report Should the type for parameter `$action` not be `string\|null`? This check looks for `@param` annotations where the type inferred by our type inference engine differs from the declared type. It makes a suggestion as to what type it considers more descriptive. Most often this is a case of a parameter that can be null in addition to its declared types. Loading history...
97		* @return string
98		*/
99		public function Link($action = null)
100		{
101		return Controller::join_links(
102		$this->config()->url_segment,
103		$action
104		);
105		}
106
107		/**
108		* Get an absolute link to this controller
109		*
110		* @param string $action
		0 ignored issues – show Documentation introduced 2018-02-21 15:59 UTC by Report Bug Copy Issue Report Should the type for parameter `$action` not be `string\|null`? This check looks for `@param` annotations where the type inferred by our type inference engine differs from the declared type. It makes a suggestion as to what type it considers more descriptive. Most often this is a case of a parameter that can be null in addition to its declared types. Loading history...
111		* @return string
		0 ignored issues – show Documentation introduced 2018-02-21 15:59 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...
112		*/
113		public function AbsoluteLink($action = null)
114		{
115		return Director::absoluteURL($this->Link($action));
116		}
117
118		/**
119		* Get a relative (to the root url of the site) link to this
120		* controller
121		*
122		* @param string $action
		0 ignored issues – show Documentation introduced 2018-02-21 15:59 UTC by Report Bug Copy Issue Report Should the type for parameter `$action` not be `string\|null`? This check looks for `@param` annotations where the type inferred by our type inference engine differs from the declared type. It makes a suggestion as to what type it considers more descriptive. Most often this is a case of a parameter that can be null in addition to its declared types. Loading history...
123		* @return string
124		*/
125		public function RelativeLink($action = null)
126		{
127		return Controller::join_links(
128		$this->Link($action)
129		);
130		}
131
132		/**
133		* Display the currently outstanding orders for the current user
134		*
135		*/
136	View Code Duplication	public function index()
		0 ignored issues – show Duplication introduced 2018-02-21 15:44 UTC by Report Bug Copy Issue Report This method seems to be duplicated in your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history...
137		{
138		$member = Member::currentUser();
		0 ignored issues – show Unused Code introduced 2018-02-21 15:44 UTC by Report Bug Copy Issue Report `$member` 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...
139
140		$this->customise(array(
141		"ClassName" => "AccountPage"
142		));
143
144		$this->extend("onBeforeIndex");
145
146		return $this->renderWith(array(
147		"UserAccount",
148		"UserAccount",
149		"Page"
150		));
151		}
152
153	View Code Duplication	public function edit()
		0 ignored issues – show Duplication introduced 2018-02-21 15:44 UTC by Report Bug Copy Issue Report This method seems to be duplicated in your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history...
154		{
155		$member = Member::currentUser();
156
157		$this->customise(array(
158		"ClassName" => "AccountPage",
159		"Form" => $this->EditAccountForm()->loadDataFrom($member)
		0 ignored issues – show Bug introduced 2018-02-21 15:44 UTC by Report Bug Copy Issue Report It seems like `$member` defined by `\Member::currentUser()` on line `155` can be `null`; however, `Form::loadDataFrom()` does not accept `null`, maybe add an additional type check? Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code: /** @return stdClass\|null */ function mayReturnNull() { } function doesNotAcceptNull(stdClass $x) { } // With potential error. function withoutCheck() { $x = mayReturnNull(); doesNotAcceptNull($x); // Potential error here. } // Safe - Alternative 1 function withCheck1() { $x = mayReturnNull(); if ( ! $x instanceof stdClass) { throw new \LogicException('$x must be defined.'); } doesNotAcceptNull($x); } // Safe - Alternative 2 function withCheck2() { $x = mayReturnNull(); if ($x instanceof stdClass) { doesNotAcceptNull($x); } } Loading history...
160		));
161
162		$this->extend("onBeforeEdit");
163
164		return $this->renderWith(array(
165		"UserAccount_edit",
166		"UserAccount",
167		"Page"
168		));
169		}
170
171		public function changepassword()
172		{
173		// Set the back URL for this form
174		$back_url = Controller::join_links(
175		$this->Link("changepassword"),
176		"?s=1"
177		);
178
179		Session::set("BackURL", $back_url);
180
181		$form = $this->ChangePasswordForm();
182
183		// Is password changed, set a session message.
184		$password_set = $this->request->getVar("s");
185		if($password_set && $password_set == 1) {
186		$form->sessionMessage(
187		_t("Users.PasswordChangedSuccessfully","Password Changed Successfully"),
188		"good"
189		);
190		}
191
192		$this->customise(array(
193		"ClassName" => "AccountPage",
194		"Form" => $form
195		));
196
197		$this->extend("onBeforeChangePassword");
198
199		return $this->renderWith(array(
200		"UserAccount_changepassword",
201		"UserAccount",
202		"Page"
203		));
204		}
205
206		/**
207		* Factory for generating a profile form. The form can be expanded using an
208		* extension class and calling the updateEditProfileForm method.
209		*
210		* @return Form
211		*/
212		public function EditAccountForm()
213		{
214		$form = Users_EditAccountForm::create($this, "EditAccountForm");
215
216		$this->extend("updateEditAccountForm", $form);
217
218		return $form;
219		}
220
221		/**
222		* Factory for generating a change password form. The form can be expanded
223		* using an extension class and calling the updateChangePasswordForm method.
224		*
225		* @return Form
226		*/
227		public function ChangePasswordForm()
228		{
229		$form = ChangePasswordForm::create($this, "ChangePasswordForm");
230
231		$form
232		->Actions()
233		->find("name", "action_doChangePassword")
234		->addExtraClass("btn")
235		->addExtraClass("btn-green");
236
237		$cancel_btn = LiteralField::create(
238		"CancelLink",
239		'<a href="' . $this->Link() . '" class="btn btn-red">'. _t("Users.CANCEL", "Cancel") .'</a>'
240		);
241
242		$form
243		->Actions()
244		->insertBefore($cancel_btn, "action_doChangePassword");
		0 ignored issues – show Documentation introduced 2018-02-21 15:44 UTC by Report Bug Copy Issue Report `'action_doChangePassword'` is of type `string`, but the function expects a `object<FormField>`. It seems like the type of the argument is not accepted by the function/method which you are calling. In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug. We suggest to add an explicit type cast like in the following example: function acceptsInteger($int) { } $x = '123'; // string "123" // Instead of acceptsInteger($x); // we recommend to use acceptsInteger((integer) $x); Loading history...
245
246		$this->extend("updateChangePasswordForm", $form);
247
248		return $form;
249		}
250
251		/**
252		* Return a list of nav items for managing a users profile. You can add new
253		* items to this menu using the "updateAccountMenu" extension
254		*
255		* @return ArrayList
256		*/
257		public function getAccountMenu()
258		{
259		$menu = new ArrayList();
260
261		$curr_action = $this->request->param("Action");
262
263		$menu->add(new ArrayData(array(
264		"ID" => 0,
265		"Title" => _t('Users.PROFILESUMMARY', "Profile Summary"),
266		"Link" => $this->Link(),
267		"LinkingMode" => (!$curr_action) ? "current" : "link"
268		)));
269
270		$menu->add(new ArrayData(array(
271		"ID" => 10,
272		"Title" => _t('Users.EDITDETAILS', "Edit account details"),
273		"Link" => $this->Link("edit"),
274		"LinkingMode" => ($curr_action == "edit") ? "current" : "link"
275		)));
276
277		$menu->add(new ArrayData(array(
278		"ID" => 30,
279		"Title" => _t('Users.CHANGEPASSWORD', "Change password"),
280		"Link" => $this->Link("changepassword"),
281		"LinkingMode" => ($curr_action == "changepassword") ? "current" : "link"
282		)));
283
284		$this->extend("updateAccountMenu", $menu);
285
286		return $menu->sort("ID", "ASC");
287		}
288
289		public function providePermissions()
290		{
291		return array(
292		"USERS_MANAGE_ACCOUNT" => array(
293		'name' => 'Manage user account',
294		'help' => 'Allow user to manage their account details',
295		'category' => 'Frontend Users',
296		'sort' => 100
297		),
298		"USERS_VERIFIED" => array(
299		'name' => 'Verified user',
300		'help' => 'Users have verified their account',
301		'category' => 'Frontend Users',
302		'sort' => 100
303		),
304		);
305		}
306		}
307

i-lateral / silverstripe-users

Push — 1 ( 1e411c...a166aa )

Users_Account_Controller A

Complexity

Size/Duplication

Coupling/Cohesion

Importance

14 Methods

How to fix Duplicated Code

Duplicated Code

Duplication Side-by-Side

Filter issues like