Conditions | 17 |
Paths | 113 |
Total Lines | 113 |
Lines | 0 |
Ratio | 0 % |
Changes | 0 |
Small methods make your code easier to understand, in particular if combined with a good name. Besides, if your method is small, finding a good name is usually much easier.
For example, if you find yourself adding comments to a method's body, this is usually a good sign to extract the commented part to a new method, and use the comment as a starting point when coming up with a good name for this new method.
Commonly applied refactorings include:
If many parameters/temporary variables are present:
1 | <?php namespace XoopsModules\Smartobject; |
||
57 | public function addAndActivateUser($userObj, $groups = false, $notifyUser = true, &$password = false) |
||
58 | { |
||
59 | $email = $userObj->getVar('email'); |
||
60 | if (!$userObj->getVar('email') || '' === $email) { |
||
61 | $userObj->setErrors(_CO_SOBJECT_USER_NEED_EMAIL); |
||
62 | |||
63 | return false; |
||
64 | } |
||
65 | |||
66 | $password = $userObj->getVar('pass'); |
||
67 | // randomly generating the password if not already set |
||
68 | if ('' === $password) { |
||
69 | $password = substr(md5(uniqid(mt_rand(), 1)), 0, 6); |
||
70 | } |
||
71 | $userObj->setVar('pass', md5($password)); |
||
72 | |||
73 | // if no username is set, let's generate one |
||
74 | $unamecount = 20; |
||
75 | $uname = $userObj->getVar('uname'); |
||
76 | if (!$uname || '' === $uname) { |
||
77 | $usernames = $this->genUserNames($email, $unamecount); |
||
78 | $newuser = false; |
||
79 | $i = 0; |
||
80 | while (false === $newuser) { |
||
81 | $crit = new \Criteria('uname', $usernames[$i]); |
||
82 | $count = $this->getUserCount($crit); |
||
83 | if (0 == $count) { |
||
84 | $newuser = true; |
||
85 | } else { |
||
86 | //Move to next username |
||
87 | ++$i; |
||
88 | if ($i == $unamecount) { |
||
89 | //Get next batch of usernames to try, reset counter |
||
90 | $usernames = $this->genUserNames($email, $unamecount); |
||
91 | $i = 0; |
||
92 | } |
||
93 | } |
||
94 | } |
||
95 | } |
||
96 | |||
97 | global $xoopsConfig; |
||
98 | |||
99 | $configHandler = xoops_getHandler('config'); |
||
100 | $xoopsConfigUser = $configHandler->getConfigsByCat(XOOPS_CONF_USER); |
||
101 | switch ($xoopsConfigUser['activation_type']) { |
||
102 | case 0: |
||
103 | $level = 0; |
||
|
|||
104 | $mailtemplate = 'smartmail_activate_user.tpl'; |
||
105 | $aInfoMessages[] = sprintf(_NL_MA_NEW_USER_NEED_ACT, $user_email); |
||
106 | break; |
||
107 | case 1: |
||
108 | $level = 1; |
||
109 | $mailtemplate = 'smartmail_auto_activate_user.tpl'; |
||
110 | $aInfoMessages[] = sprintf(_NL_MA_NEW_USER_AUTO_ACT, $user_email); |
||
111 | break; |
||
112 | case 2: |
||
113 | default: |
||
114 | $level = 0; |
||
115 | $mailtemplate = 'smartmail_admin_activate_user.tpl'; |
||
116 | $aInfoMessages[] = sprintf(_NL_MA_NEW_USER_ADMIN_ACT, $user_email); |
||
117 | } |
||
118 | |||
119 | $userObj->setVar('uname', $usernames[$i]); |
||
120 | $userObj->setVar('user_avatar', 'blank.gif'); |
||
121 | $userObj->setVar('user_regdate', time()); |
||
122 | $userObj->setVar('timezone_offset', $xoopsConfig['default_TZ']); |
||
123 | $actkey = substr(md5(uniqid(mt_rand(), 1)), 0, 8); |
||
124 | $userObj->setVar('actkey', $actkey); |
||
125 | $userObj->setVar('email', $email); |
||
126 | $userObj->setVar('notify_method', 2); |
||
127 | $userObj->setVar('level', $userObj); |
||
128 | |||
129 | if ($this->insertUser($userObj)) { |
||
130 | |||
131 | // if $groups=false, Add the user to Registered Users group |
||
132 | if (!$groups) { |
||
133 | $this->addUserToGroup(XOOPS_GROUP_USERS, $userObj->getVar('uid')); |
||
134 | } else { |
||
135 | foreach ($groups as $groupid) { |
||
136 | $this->addUserToGroup($groupid, $userObj->getVar('uid')); |
||
137 | } |
||
138 | } |
||
139 | } else { |
||
140 | return false; |
||
141 | } |
||
142 | |||
143 | if ($notifyUser) { |
||
144 | // send some notifications |
||
145 | $xoopsMailer = xoops_getMailer(); |
||
146 | $xoopsMailer->useMail(); |
||
147 | $xoopsMailer->setTemplateDir(SMARTOBJECT_ROOT_PATH . 'language/' . $xoopsConfig['language'] . '/mail_template'); |
||
148 | $xoopsMailer->setTemplate('smartobject_notify_user_added_by_admin.tpl'); |
||
149 | $xoopsMailer->assign('XOOPS_USER_PASSWORD', $password); |
||
150 | $xoopsMailer->assign('SITENAME', $xoopsConfig['sitename']); |
||
151 | $xoopsMailer->assign('ADMINMAIL', $xoopsConfig['adminmail']); |
||
152 | $xoopsMailer->assign('SITEURL', XOOPS_URL . '/'); |
||
153 | $xoopsMailer->assign('NAME', $userObj->getVar('name')); |
||
154 | $xoopsMailer->assign('UNAME', $userObj->getVar('uname')); |
||
155 | $xoopsMailer->setToUsers($userObj); |
||
156 | $xoopsMailer->setFromEmail($xoopsConfig['adminmail']); |
||
157 | $xoopsMailer->setFromName($xoopsConfig['sitename']); |
||
158 | $xoopsMailer->setSubject(sprintf(_CO_SOBJECT_NEW_USER_NOTIFICATION_SUBJECT, $xoopsConfig['sitename'])); |
||
159 | |||
160 | if (!$xoopsMailer->send(true)) { |
||
161 | /** |
||
162 | * @todo trap error if email was not sent |
||
163 | */ |
||
164 | $xoopsMailer->getErrors(true); |
||
165 | } |
||
166 | } |
||
167 | |||
168 | return true; |
||
169 | } |
||
170 | |||
264 |
This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently.
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.