Complex classes like CrudController often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.
Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.
While breaking up the class, it is a good idea to analyze how other classes use CrudController, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
10 | class CrudController extends Controller |
||
11 | { |
||
12 | protected $route; |
||
13 | protected $entity; |
||
14 | protected $entityInstance = null; |
||
15 | protected $fields = []; |
||
16 | protected $columns = []; |
||
17 | protected $buttons = [ |
||
18 | 'create', |
||
19 | 'edit', |
||
20 | 'delete', |
||
21 | ]; |
||
22 | protected $paginate = null; |
||
23 | protected $searchable = []; |
||
24 | protected $filters = []; |
||
25 | protected $queryFilters = []; |
||
26 | protected $filterRequire = []; |
||
27 | protected $textsGeneral = [ |
||
28 | 'list_title' => 'Contents', |
||
29 | 'create_title' => '', |
||
30 | 'edit_title' => '', |
||
31 | ]; |
||
32 | protected $texts = []; |
||
33 | protected $htmlFilters = []; |
||
34 | protected $action = null; |
||
35 | protected $formColsClasses = [ |
||
36 | 'col-md-10 col-md-offset-1', |
||
37 | 'col-md-2', |
||
38 | 'col-md-10', |
||
39 | ]; |
||
40 | protected $links = []; |
||
41 | |||
42 | public function index(Request $request) |
||
76 | |||
77 | public function create() |
||
89 | |||
90 | public function store(Request $request) |
||
91 | { |
||
92 | $this->validateRequest($request); |
||
93 | |||
94 | DB::beginTransaction(); |
||
95 | |||
96 | try { |
||
97 | $row = $this->entity->create(array_merge($request->all(), $this->queryFilters)); |
||
98 | $this->updateForeignRelations($row, $request); |
||
99 | } catch (QueryException $e) { |
||
100 | return redirect() |
||
101 | ->back() |
||
102 | ->with('error', 'Ha ocurrido un error, intente nuevamente'); |
||
103 | } |
||
104 | |||
105 | DB::commit(); |
||
106 | |||
107 | return redirect() |
||
108 | ->route($this->route.'.index') |
||
109 | ->with('success', isset($this->textsGeneral['save_action']) |
||
110 | ? $this->textsGeneral['save_action'] |
||
111 | : trans('eliurkis::crud.save_action')); |
||
112 | } |
||
113 | |||
114 | public function edit($id) |
||
115 | { |
||
116 | if (! $this->entityInstance) { |
||
117 | $this->entityInstance = $this->entity->findOrFail($id); |
||
118 | } |
||
119 | |||
120 | $this->prepareFields(); |
||
121 | |||
122 | return view('crud::create') |
||
123 | ->with('type', 'edit') |
||
124 | ->with('route', $this->route) |
||
125 | ->with('t', $this->texts) |
||
126 | ->with('fields', $this->fields) |
||
127 | ->with('formColsClasses', $this->formColsClasses) |
||
128 | ->with('links', $this->prepareLinks()) |
||
129 | ->with('data', $this->entityInstance); |
||
130 | } |
||
131 | |||
132 | public function update(Request $request, $id) |
||
133 | { |
||
134 | $this->validateRequest($request); |
||
135 | |||
136 | DB::beginTransaction(); |
||
137 | |||
138 | try { |
||
139 | $row = $this->entity->findOrFail($id); |
||
140 | $row->update( |
||
141 | array_merge( |
||
142 | $request->all(), |
||
143 | $this->queryFilters |
||
144 | ) |
||
145 | ); |
||
146 | $this->updateForeignRelations($row, $request); |
||
147 | } catch (QueryException $e) { |
||
148 | return redirect() |
||
149 | ->back() |
||
150 | ->with('error', 'Ha ocurrido un error, intente nuevamente'); |
||
151 | } |
||
152 | |||
153 | DB::commit(); |
||
154 | |||
155 | return redirect() |
||
156 | ->route($this->route.'.index', $this->getParamsFilters($row)) |
||
157 | ->with('success', isset($this->textsGeneral['save_action']) |
||
158 | ? $this->textsGeneral['save_action'] |
||
159 | : trans('eliurkis::crud.save_action')); |
||
160 | } |
||
161 | |||
162 | public function destroy($id) |
||
163 | { |
||
164 | $this->entity->destroy($id); |
||
165 | |||
166 | return redirect() |
||
167 | ->route($this->route.'.index') |
||
168 | ->with('success', isset($this->textsGeneral['delete_action']) |
||
169 | ? $this->textsGeneral['delete_action'] |
||
170 | : trans('eliurkis::crud.delete_action')); |
||
171 | } |
||
172 | |||
173 | /* Private Actions */ |
||
174 | |||
175 | /** |
||
176 | * @param $entity |
||
177 | * @param Request $request |
||
178 | * |
||
179 | * @return mixed |
||
180 | */ |
||
181 | protected function filters($entity, $request) |
||
197 | |||
198 | protected function htmlFilters() |
||
224 | |||
225 | /** |
||
226 | * @param $entity |
||
227 | * @param Request $request |
||
228 | * |
||
229 | * @return mixed |
||
230 | */ |
||
231 | protected function paginate($entity, $request) |
||
247 | |||
248 | /** |
||
249 | * @param $entity |
||
250 | * @param Request $request |
||
251 | * |
||
252 | * @return mixed |
||
253 | */ |
||
254 | protected function search($entity, $request) |
||
274 | |||
275 | protected function getForeignRelationsFields() |
||
286 | |||
287 | protected function getBelongToFields() |
||
298 | |||
299 | /** |
||
300 | * @param object $row |
||
301 | * @param Request $request |
||
302 | */ |
||
303 | protected function updateForeignRelations($row, $request) |
||
312 | |||
313 | protected function getParamsFilters($row) |
||
327 | |||
328 | protected function prepareLinks() |
||
340 | |||
341 | /** |
||
342 | * @param Request $request |
||
343 | */ |
||
344 | protected function validateRequest($request) |
||
368 | |||
369 | protected function prepareRelationalFields($name) |
||
394 | |||
395 | protected function prepareFields() |
||
405 | |||
406 | protected function prepareField($name, $properties = []) |
||
455 | } |
||
456 |
There are different options of fixing this problem.
If you want to be on the safe side, you can add an additional type-check:
If you are sure that the expression is traversable, you might want to add a doc comment cast to improve IDE auto-completion and static analysis:
Mark the issue as a false-positive: Just hover the remove button, in the top-right corner of this issue for more options.