D
D
Dmitry2018-03-15 19:16:21
Yii
Dmitry, 2018-03-15 19:16:21

Why is this done, what can be done with the code next?

Good afternoon.
They contacted me with a proposal to make some changes on the site, which is developed on yii2.
While it was required to configure urlManager(), but there will be a bunch of other edits.
Website for the sale / hire of cars.
Looking into one of the controllers, I'm a little "hung". A controller of 1,500 lines, most of the actions in it receive information from the database about cars, but with different parameters.
Here is an example of such an action, choosing from a database of cars in a given region.

actionSearchByRegion
public function actionSearchByRegion($region = null)
    {
        if (\Yii::$app->request->isGet) {

            $region = \Yii::$app->request->get('region-name');
            $region = str_replace('-', ' ', $region);
//            $region = substr($region, 0, 5);
            $region = Regions::find()
                ->select('region_id')
                ->where(['like', 'region_name', $region])
                ->asArray()
                ->one();

            if (!empty($region)) {
                $cars = Cars::find()
                    ->where(['=', 'car_region', $region['region_id']])
                    ->andWhere(['=', 'car_sold', 0])
                    ->orderBy(['id' => SORT_DESC])
                    ->asArray();

                $query = $cars;
                $countQuery = clone $query;
                $pages = new Pagination([
                    'totalCount' => $countQuery->count(),
                    'defaultPageSize' => 12,
                ]);
                $models = $query->offset($pages->offset)->limit($pages->limit)->all();

                return $this->render('search', [
                    'cars_res' => $models,
                    'pages' => $pages,
                ]);
            }

        }
    }


The next action is actionSearchByCity , in which cars with a given city are received. There is also actionSearchByRegionCityMark and actionSearchByRegionCityMarkModel , in which you need to get cars of this model and brand associated with a given region and city, respectively.
There is no search model as such, there is, again, a separate action for searching.
There are no links between models, no foreign keys. At the same time, the debug panel shows over 150 queries to the database. Many requests are duplicated.

In the views that this controller connects, there is, in my opinion, quite "beautiful" code.
<?= $form->field($searchcar, 'city')->label('Okres')->dropDownList(
//                            ArrayHelper::map(\backend\models\City::find()->where(['region_id' => $searchcar->state])->asArray()->all(), 'city_id', 'city_name'),
                                ArrayHelper::map(\backend\models\City::find()->where(['region_id' => $_GET['SearchCar']['state'] ? $_GET['SearchCar']['state'] : $_GET['region']])->asArray()->all(), 'city_id', 'city_name'),
                                [
                                    'prompt' => 'Vyberte okres',
                                    ///'value' => $_GET['SearchCar']['city'] ? $_GET['SearchCar']['city'] : $_GET['city']
                                ]
                            ) ?>

Here is the controller code in full . Look, please, with an experienced eye and express your opinion.
I don’t have much experience, so I would like to know the opinion of more experienced people, what should I do with this code?
Should I leave it as it is and make minor changes, or should I suggest rewriting?
What exactly did the previous developer want to achieve with such code (lack of relationships, foreign keys, etc.)?
What should I tell the customer about the edits?

Answer the question

In order to leave comments, you need to log in

1 answer(s)
V
Vitaly Khomenko, 2018-03-15
@slo_nik

I do not defend this code and I do not like it at all, but there is a fictional ideal world and there is a real one. In the real world, this happens very often.
To do nothing. You can try to explain the problem to the client, but most likely he will refuse to redo the project. Are you sure that with your "not very much experience" you can quickly and efficiently rewrite? Why does the client need to pay a second time if he does not see any changes from the UI side?
Do your job and inform the client about problems that will become even bigger in the future.
He implemented the task using his experience and capabilities, and received money for it. He didn't try to make the world a better place.

Didn't find what you were looking for?

Ask your question

Ask a Question

731 491 924 answers to any question