Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.
Common duplication problems, and corresponding solutions are:
1 | <?php |
||
13 | class MakeUser extends Command |
||
14 | { |
||
15 | /** |
||
16 | * The name and signature of the console command. |
||
17 | * |
||
18 | * @var string |
||
19 | */ |
||
20 | protected $signature = 'make:user |
||
21 | {--email= : Set the email for the new user} |
||
22 | {--name= : Set the name for the new user} |
||
23 | {--password= : The password to set for the new user} |
||
24 | {--send-reset : Send a password reset email for the new user} |
||
25 | {--fields= : Additional database fields to set on the user} |
||
26 | {--force : Create the user model circumventing guarded fields} |
||
27 | {--import-file= : Relative path and filename for a file to import users from. File name MUST contain the extension representing the type of file (Ex: ./path/to/file.csv)} |
||
28 | '; |
||
29 | |||
30 | /** |
||
31 | * The console command description. |
||
32 | * |
||
33 | * @var string |
||
34 | */ |
||
35 | protected $description = 'Create new application users'; |
||
36 | |||
37 | /** |
||
38 | * Execute the console command. |
||
39 | * |
||
40 | * Handle creation of the new application user. |
||
41 | * |
||
42 | * @return void |
||
43 | */ |
||
44 | 27 | public function handle() |
|
45 | { |
||
46 | 27 | $dataToProcess = []; |
|
47 | |||
48 | try { |
||
49 | 27 | $bulkImportFile = is_string($this->option('import-file')) ? $this->fileHandlerFactory($this->option('import-file')) : null; |
|
50 | |||
51 | 21 | $modelCommand = $this->option('force') ? 'forceCreate' : 'create'; |
|
52 | |||
53 | 21 | if (! is_null($bulkImportFile)) { |
|
54 | 12 | $dataToProcess = $bulkImportFile->getData(); |
|
55 | |||
56 | 12 | $sendReset = false; |
|
57 | |||
58 | 12 | if (! in_array('password', array_keys($dataToProcess[0])) || $this->option('send-reset')) { |
|
59 | 4 | $sendReset = true; |
|
60 | } |
||
61 | 12 | $dataToProcess = $this->setPasswords($dataToProcess); |
|
62 | |||
63 | } else { |
||
64 | 9 | $email = $this->option('email'); |
|
65 | 9 | $name = $this->option('name') ?: ''; |
|
66 | 9 | $password = bcrypt($this->option('password') ?: str_random(32)); |
|
67 | 9 | $sendReset = ! $this->option('password') || $this->option('send-reset'); |
|
68 | |||
69 | 9 | $dataToProcess[0] = [ |
|
70 | 9 | 'email' => $email, |
|
71 | 9 | 'name' => $name, |
|
72 | 9 | 'password' => $password, |
|
73 | ]; |
||
74 | } |
||
75 | |||
76 | 21 | foreach ($dataToProcess as $dataRow) { |
|
77 | |||
78 | 21 | $email = $dataRow['email'] ?? null; |
|
79 | |||
80 | 21 | app('db')->beginTransaction(); |
|
81 | |||
82 | 21 | $this->validateEmail($email); |
|
83 | |||
84 | 17 | app(config('auth.providers.users.model'))->{$modelCommand}(array_merge( |
|
85 | 17 | $dataRow, |
|
86 | 17 | $bulkImportFile ? [] : $this->additionalFields() |
|
87 | )); |
||
88 | |||
89 | 17 | if ($sendReset) { |
|
90 | 7 | Password::sendResetLink(compact('email')); |
|
91 | } |
||
92 | |||
93 | 17 | app('db')->commit(); |
|
94 | } |
||
95 | |||
96 | 17 | if (count($dataToProcess)) { |
|
97 | 17 | $createdMessage = $bulkImportFile |
|
98 | 10 | ? 'Created '.count($dataToProcess).' user(s).' |
|
99 | 17 | : "Created new user for email {$email}."; |
|
|
|||
100 | |||
101 | 17 | $passwordResetMessage = $bulkImportFile |
|
102 | 10 | ? 'Sent password reset emails.' |
|
103 | 17 | : "Sent password reset email to {$email}."; |
|
104 | |||
105 | 17 | $this->info($createdMessage); |
|
106 | |||
107 | 17 | if ($sendReset) { |
|
108 | 17 | $this->info($passwordResetMessage); |
|
109 | } |
||
110 | |||
111 | } else { |
||
112 | 17 | $this->error('The user(s) were not created'); |
|
113 | } |
||
114 | |||
115 | 10 | } catch (Exception $e) { |
|
116 | 10 | $this->error($e->getMessage()); |
|
117 | |||
118 | 10 | $this->error('The user(s) were not created'); |
|
119 | |||
120 | 10 | app('db')->rollBack(); |
|
121 | } |
||
122 | 27 | } |
|
123 | |||
124 | /** |
||
125 | * Determine if the given email address already exists. |
||
126 | * |
||
127 | * @param string $email |
||
128 | * @return void |
||
129 | * |
||
130 | * @throws \Dyrynda\Artisan\Exceptions\MakeUserException |
||
131 | */ |
||
132 | 21 | private function validateEmail($email) |
|
142 | |||
143 | /** |
||
144 | * Return any additional database fields passed by the --fields option. |
||
145 | * |
||
146 | * @return array |
||
147 | */ |
||
148 | 7 | private function additionalFields() |
|
160 | |||
161 | /** |
||
162 | * Normalise the given (database) field input value. |
||
163 | * |
||
164 | * @param mixed $value |
||
165 | * @return mixed |
||
166 | */ |
||
167 | 3 | private function normaliseValue($value) |
|
183 | |||
184 | /** |
||
185 | * Create file handler objects. |
||
186 | * |
||
187 | * @param string $path |
||
188 | * @return BulkImportFileHandler |
||
189 | * |
||
190 | * @throws \Dyrynda\Artisan\Exceptions\ImportFileException |
||
191 | */ |
||
192 | 18 | private function fileHandlerFactory($path) : BulkImportFileHandler |
|
206 | |||
207 | /** |
||
208 | * Add default password to data. |
||
209 | * |
||
210 | * @param array $data |
||
211 | * @return array |
||
212 | */ |
||
213 | private function setPasswords($data) |
||
222 | } |
||
223 |
If you define a variable conditionally, it can happen that it is not defined for all execution paths.
Let’s take a look at an example:
In the above example, the variable $x is defined if you pass “foo” or “bar” as argument for $a. However, since the switch statement has no default case statement, if you pass any other value, the variable $x would be undefined.
Available Fixes
Check for existence of the variable explicitly:
Define a default value for the variable:
Add a value for the missing path: