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 |
||
30 | class ProjectController extends Controller |
||
31 | { |
||
32 | /** |
||
33 | * Display activity for a project. |
||
34 | * |
||
35 | * @param Project $project |
||
36 | * |
||
37 | * @return \Illuminate\View\View |
||
38 | */ |
||
39 | 11 | public function getIndex(Project $project) |
|
40 | { |
||
41 | 11 | $activities = $project->activities() |
|
42 | 11 | ->with('activity', 'issue', 'user', 'assignTo', 'comment', 'note') |
|
43 | 11 | ->orderBy('users_activity.created_at', 'DESC') |
|
44 | 11 | ->take(10); |
|
45 | |||
46 | // Internal project and logged user can see created only |
||
47 | 11 | if ($project->isPrivateInternal() && $this->auth->user()->isUser()) { |
|
|
|||
48 | $activities->join('projects_issues', 'projects_issues.id', '=', 'item_id'); |
||
49 | $activities->where('created_by', '=', $this->auth->user()->id); |
||
50 | } |
||
51 | |||
52 | 11 | return view('project.index', [ |
|
53 | 11 | 'tabs' => $this->projectMainViewTabs($project, 'index'), |
|
54 | 11 | 'project' => $project, |
|
55 | 11 | 'active' => 'activity', |
|
56 | 11 | 'activities' => $activities->get(), |
|
57 | 11 | 'sidebar' => 'project', |
|
58 | ]); |
||
59 | } |
||
60 | |||
61 | /** |
||
62 | * Display issues for a project. |
||
63 | * |
||
64 | * @param FilterForm $filterForm |
||
65 | * @param Request $request |
||
66 | * @param Project $project |
||
67 | * @param int $status |
||
68 | * |
||
69 | * @return \Illuminate\View\View |
||
70 | */ |
||
71 | 2 | public function getIssues(FilterForm $filterForm, Request $request, Project $project, $status = Issue::STATUS_OPEN) |
|
72 | { |
||
73 | 2 | $request['created_by'] = auth()->user()->id; |
|
74 | 2 | $active = $status == Issue::STATUS_OPEN ? 'open_issue' : 'closed_issue'; |
|
75 | 2 | $issues = $project->listIssues($status, $request->all()); |
|
76 | |||
77 | 2 | return view('project.index', [ |
|
78 | 2 | 'tabs' => $this->projectMainViewTabs($project, 'issues', $issues, $status), |
|
79 | 2 | 'project' => $project, |
|
80 | 2 | 'active' => $active, |
|
81 | 2 | 'issues' => $issues, |
|
82 | 2 | 'sidebar' => 'project', |
|
83 | 2 | 'filterForm' => $filterForm, |
|
84 | ]); |
||
85 | } |
||
86 | |||
87 | /** |
||
88 | * Display issues assigned to current user for a project. |
||
89 | * |
||
90 | * @param Project $project |
||
91 | * |
||
92 | * @return \Illuminate\View\View |
||
93 | */ |
||
94 | 1 | View Code Duplication | public function getAssigned(Project $project) |
1 ignored issue
–
show
|
|||
95 | { |
||
96 | 1 | $issues = $project->listAssignedOrCreatedIssues($this->auth->user()); |
|
97 | |||
98 | 1 | return view('project.index', [ |
|
99 | 1 | 'tabs' => $this->projectMainViewTabs($project, 'assigned', $issues), |
|
100 | 1 | 'project' => $project, |
|
101 | 1 | 'active' => 'issue_assigned_to_you', |
|
102 | 1 | 'issues' => $issues, |
|
103 | 1 | 'sidebar' => 'project', |
|
104 | ]); |
||
105 | } |
||
106 | |||
107 | /** |
||
108 | * Display issues created to current user for a project. |
||
109 | * |
||
110 | * @param Project $project |
||
111 | * |
||
112 | * @return \Illuminate\View\View |
||
113 | */ |
||
114 | View Code Duplication | public function getCreated(Project $project) |
|
126 | |||
127 | /** |
||
128 | * Display notes for a project. |
||
129 | * |
||
130 | * @param Project $project |
||
131 | * @param NoteForm $form |
||
132 | * |
||
133 | * @return \Illuminate\View\View |
||
134 | */ |
||
135 | 7 | public function getNotes(Project $project, NoteForm $form) |
|
136 | { |
||
137 | 7 | $notes = $project->notes()->with('createdBy')->get(); |
|
138 | |||
139 | 7 | return view('project.index', [ |
|
140 | 7 | 'tabs' => $this->projectMainViewTabs($project, 'notes', $notes), |
|
141 | 7 | 'project' => $project, |
|
142 | 7 | 'active' => 'notes', |
|
143 | 7 | 'notes' => $notes, |
|
144 | 7 | 'sidebar' => 'project', |
|
145 | 7 | 'noteForm' => $form, |
|
146 | ]); |
||
147 | } |
||
148 | |||
149 | /** |
||
150 | * @param Project $project |
||
151 | * @param string $view |
||
152 | * @param null $data |
||
153 | * @param bool $status |
||
154 | * |
||
155 | * @return array |
||
156 | */ |
||
157 | 21 | protected function projectMainViewTabs(Project $project, $view, $data = null, $status = false) |
|
158 | { |
||
159 | 21 | $notesCount = $view === 'note' ? $data->count() : $project->notes()->count(); |
|
160 | 21 | $user = $this->auth->user(); |
|
161 | 21 | $isLoggedIn = !$this->auth->guest(); |
|
162 | 21 | $isUser = $isLoggedIn && $user->isUser(); |
|
163 | 21 | $isInternalProject = $project->isPrivateInternal(); |
|
164 | |||
165 | 21 | if ($view === 'issues') { |
|
166 | 2 | if ($status == Issue::STATUS_OPEN) { |
|
167 | 2 | $closedIssuesCount = $project->closedIssuesCount($user)->count(); |
|
168 | 2 | $openIssuesCount = $data->count(); |
|
169 | } else { |
||
170 | 1 | $closedIssuesCount = $data->count(); |
|
171 | 2 | $openIssuesCount = $project->openIssuesCount($user)->count(); |
|
172 | } |
||
173 | } else { |
||
174 | 19 | $openIssuesCount = $project->openIssuesCount($user)->count(); |
|
175 | 19 | $closedIssuesCount = $project->closedIssuesCount($user)->count(); |
|
176 | } |
||
177 | |||
178 | 21 | $tabs = []; |
|
179 | 21 | $tabs[] = [ |
|
180 | 21 | 'url' => $project->to(), |
|
181 | 21 | 'page' => 'activity', |
|
182 | ]; |
||
183 | 21 | $tabs[] = [ |
|
184 | 21 | 'url' => $project->to('issues'), |
|
185 | 21 | 'page' => 'open_issue', |
|
186 | 21 | 'prefix' => $openIssuesCount, |
|
187 | ]; |
||
188 | 21 | $tabs[] = [ |
|
189 | 21 | 'url' => $project->to('issues') . '/0', |
|
190 | 21 | 'page' => 'closed_issue', |
|
191 | 21 | 'prefix' => $closedIssuesCount, |
|
192 | ]; |
||
193 | 21 | if ($isLoggedIn && (!$isInternalProject || (!$isUser && $isInternalProject))) { |
|
194 | 20 | if ($view !== 'assigned') { |
|
195 | 19 | $method = $isUser ? 'createdIssuesCount' : 'assignedIssuesCount'; |
|
196 | 19 | $assignedIssuesCount = $this->auth->user()->$method($project->id); |
|
197 | } else { |
||
198 | 1 | $assignedIssuesCount = $data->count(); |
|
199 | } |
||
200 | |||
201 | 20 | $tabs[] = [ |
|
202 | 20 | 'url' => $project->to($isUser ? 'created' : 'assigned'), |
|
203 | 20 | 'page' => ($isUser ? 'issue_created_by_you' : 'issue_assigned_to_you'), |
|
204 | 20 | 'prefix' => $assignedIssuesCount, |
|
205 | ]; |
||
206 | } |
||
207 | 21 | $tabs[] = [ |
|
208 | 21 | 'url' => $project->to('notes'), |
|
209 | 21 | 'page' => 'notes', |
|
210 | 21 | 'prefix' => $notesCount, |
|
211 | ]; |
||
212 | |||
213 | 21 | return $tabs; |
|
214 | } |
||
215 | |||
216 | /** |
||
217 | * Edit the project. |
||
218 | * |
||
219 | * @param Project $project |
||
220 | * @param Form $form |
||
221 | * |
||
222 | * @return \Illuminate\View\View |
||
223 | */ |
||
224 | 2 | public function getEdit(Project $project, Form $form) |
|
232 | |||
233 | /** |
||
234 | * To update project details. |
||
235 | * |
||
236 | * @param Project $project |
||
237 | * @param FormRequest\Project $request |
||
238 | * |
||
239 | * @return \Illuminate\Http\RedirectResponse |
||
240 | */ |
||
241 | 2 | public function postEdit(Project $project, FormRequest\Project $request) |
|
256 | |||
257 | /** |
||
258 | * Ajax: returns list of users that are not in the project. |
||
259 | * |
||
260 | * @param Project $project |
||
261 | * |
||
262 | * @return \Symfony\Component\HttpFoundation\Response |
||
263 | */ |
||
264 | 1 | public function getInactiveUsers(Project $project = null) |
|
265 | { |
||
266 | 1 | $users = $project->usersNotIn(); |
|
267 | |||
268 | 1 | return response()->json($users); |
|
269 | } |
||
270 | |||
271 | /** |
||
272 | * Ajax: add user to the project. |
||
273 | * |
||
274 | * @param Project $project |
||
275 | * @param Request $request |
||
276 | * |
||
277 | * @return \Symfony\Component\HttpFoundation\Response |
||
278 | */ |
||
279 | 1 | View Code Duplication | public function postAssign(Project $project, Request $request) |
289 | |||
290 | /** |
||
291 | * Ajax: remove user from the project. |
||
292 | * |
||
293 | * @param Project $project |
||
294 | * @param Request $request |
||
295 | * |
||
296 | * @return \Symfony\Component\HttpFoundation\Response |
||
297 | */ |
||
298 | 1 | View Code Duplication | public function postUnassign(Project $project, Request $request) |
308 | |||
309 | /** |
||
310 | * To add a new note to the project. |
||
311 | * |
||
312 | * @param Project $project |
||
313 | * @param Note $note |
||
314 | * @param FormRequest\Note $request |
||
315 | * |
||
316 | * @return \Illuminate\Http\RedirectResponse |
||
317 | */ |
||
318 | 2 | public function postAddNote(Project $project, Note $note, FormRequest\Note $request) |
|
326 | |||
327 | /** |
||
328 | * Ajax: To update project note. |
||
329 | * |
||
330 | * @param Project $project |
||
331 | * @param Note $note |
||
332 | * @param Request $request |
||
333 | * |
||
334 | * @return \Symfony\Component\HttpFoundation\Response |
||
335 | */ |
||
336 | 1 | public function postEditNote(Project $project, Project\Note $note, Request $request) |
|
348 | |||
349 | /** |
||
350 | * Ajax: to delete a project note. |
||
351 | * |
||
352 | * @param Project $project |
||
353 | * @param Note $note |
||
354 | * |
||
355 | * @return \Symfony\Component\HttpFoundation\Response |
||
356 | */ |
||
357 | 1 | public function getDeleteNote(Project $project, Project\Note $note) |
|
363 | |||
364 | /** |
||
365 | * Ajax: generate the issues export file. |
||
366 | * |
||
367 | * @param Project $project |
||
368 | * @param Exporter $exporter |
||
369 | * @param Request $request |
||
370 | * |
||
371 | * @return \Symfony\Component\HttpFoundation\Response |
||
372 | */ |
||
373 | 4 | public function postExportIssues(Project $project, Exporter $exporter, Request $request) |
|
396 | |||
397 | /** |
||
398 | * Download and then delete an export file. |
||
399 | * |
||
400 | * @param Project $project |
||
401 | * @param string $file |
||
402 | * |
||
403 | * @return \Symfony\Component\HttpFoundation\BinaryFileResponse |
||
404 | */ |
||
405 | 4 | public function getDownloadExport(Project $project, $file) |
|
413 | } |
||
414 |
Let’s take a look at an example:
In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.
Available Fixes
Change the type-hint for the parameter:
Add an additional type-check:
Add the method to the interface: