Good day. If it's not hard to look, please, this function and tell me what I did wrong in it, what can be used and what can be improved. I will be very grateful. I apologize in advance for stopping hacks for a terrible ... This function removes items from the base

public function select_deleteAll($type)//Удаление выбранных продуктов { //$type - содержит ключ для массива с данными которого будем работать $res_varchar = 0; //Резервная переменная что бы нотисы не вылетали $data = array(); //Тоже самое что и выше но только массив $data_my = array( //В этом массиве находятся все данные с которыми мы будем работать 'warehouse' => array('dbs' => 'myshop_warehouse', 'cmd' => 'warehouse_delete'), //warehouse - ключ по которому будут выдираться данные //dbs - название базы данных с которой будем работать //cmd - ниже у нас идет подключение шаблона а cmd указывает какой блок с сообщениями задействовать 'brand' => array('dbs' => 'myshop_brands', 'cmd' => 'brands_delete'), 'product' => array( 'dbs' => 'myshop_products', 'cmd' => 'product_delete', 'cycle' => array( //Указывает что у нас будет задействован цикл на вложеные массивы array( 'spec_type' => 'properties_data', // Это название основного ключа из которого мы будем выдирать dbs 'spec_type_data' => 'delete', //тип действий: update - обновление данных, delete - удаление данных, update_delete - (удаление и обновление) или (обновление или удаление) 'first' => 'item_id', // обычно тут мы указываем поле с которым будем работать ), ), ), 'category' => array( //Это ключ для работы с таблицей myshop_category 'dbs' => 'myshop_category', 'cmd' => 'category_delete', 'cycle' => array( array( //В этом блоке мы будем работать с таблицей myshop_products 'spec_type' => 'product', //Тут мы указываем что будем работать с данными массива у которого ключ product 'spec_type_data' => 'update', //myshop_products мы будем обновлять 'first' => 'category_id', //И обновим мы поле category_id 'first_data' => '0', //Вот тут обычно находится значение на которое нужно обновить, в данном случае обновим поле category_id на значение 0 ), array( 'spec_type' => array('update' => 'properties', 'delete' => 'properties_data'), //Тут мы указываем что будем работать с 2мя базами это myshop_properties и myshop_properties_data //Сначала мы обновим таблицу myshop_properties а потом удалим нужные данные из таблицы myshop_properties_data 'spec_type_data' => 'update_delete', //Тут мы указываем что будем обновлять и удалять данные 'first' => 'category', // указываем что в myshop_properties мы будем обновлять поле category 'second' => 'id', //тут мы указываем из какого поля получать данные в таблице myshop_properties для того что бы потом удалить по id данные из myshop_properties_data 'secod_data' => 'properties_id', //А вот тут мы указываем что значение id из myshop_properties соответствует properties_id из myshop_properties_data ), ), ), 'banner' => array('dbs' => 'myshop_banners', 'cmd' => 'banner_delete'), 'delivery' => array('dbs' => 'myshop_delivery', 'cmd' => 'delivery_delete'), 'payment' => array('dbs' => 'myshop_payment', 'cmd' => 'payment_delete'), 'properties' => array('dbs' => 'myshop_properties', 'cmd' => 'properties_delete'), 'properties_data' => array('dbs' => 'myshop_properties_data', 'cmd' => 'properties_data_delete'), ); if (sizeof($data_my[$type]) < 2) return false; // Сразу проверяем что в массиве с данными минимум 2 ключа, иначе уходим $data[$data_my[$type]['cmd']] = true; //Заранее активируем блок с сообщениями $ids = $this->input->post('ids'); //А вот тут у нас массив с id элементов которые мы будем использовать foreach ($ids as $id) //Запускаем цикл и обрабатываем все id { if ($id > 0) //Мало ли что, пустые значения заноситься не будут а вот 0 может влезть, по этому проверяем { $this->db->where('id', $id); //Все таблицы имеют поле id $this->db->delete($data_my[$type]['dbs']); //Тут мы используем наш массив и получаем базу данных из которой мы удалим полученный из цикла ID объекта $data['updated'] = true; //Допустим удаление прошло успешно и в выбранном блоке выведется сообщение о том что все хорошо if (isset($data_my[$type]['cycle'])) // Тут мы проверяем существует ли ключ cycle, если да, тот помимо стандартного удаления мы будем делать что то еще { foreach ($data_my[$type]['cycle'] as $prop) //Запускаем цикл и собираем инфу из блоков в cycle {//для примера давайте поработаем с ключом 'category' if ($prop['spec_type_data'] == 'update')// Если в $data_my['category']['cycle']['spec_type_data'] - передается значение update, значит будем что то обновлять (с)кэп { $data_sync = array( $prop['first'] => $prop['first_data'], //ключ first несет в себе значение category_id а first_data 0 таким образом мы будем обновлять category_id на 0 ); $this->db->where($prop['first'], $id);// WHERE category_id = $id $sync = $data_my[$prop['spec_type']]['dbs']; //Тут мы получаем название базы с которой работаем в данном случае myshop_products вот так выглядит не сокращенный вызов $data_my[ ['category']['cycle']['spec_type'] ]['dbs'] $this->db->update($sync, $data_sync); //обновляем } if ($prop['spec_type_data'] == 'delete') {//тут тоже самое что и сверху но в category мы не используем delete $this->db->where($prop['first'], $id); $db_sync = $data_my[$prop['spec_type']]['dbs']; $this->db->delete($db_sync); } if ($prop['spec_type_data'] === 'update_delete') //тут мы будем работать с (удалением и обновлением) или (удалением или обновлением) { if (isset($prop['spec_type']['update'])) //запрашиваем есть ли у нас массив $data_my['category']['cycle']['spec_type']['update'] если нет, то и делать тут нечего { $this->db->like($prop['first'], $id.',', 'both'); //first - несет в себе поле category и в нем данные в таком виде: 66,32,14 - т.е id категорий к которым привязана строка $sync = $data_my[$prop['spec_type']['update']]['dbs']; //Получаем базу с которой будем работать полный запрос выглядит так: $data_my[$data_my['category']['cycle']['spec_type']['update'] ]['dbs'] $sync = $this->db->get($sync)->result();//Ищем есть ли у нас совпадения по запросу, если нет то и делать нечего не надо // foreach ($sync as $db_sync) //Если что то есть то запускаем цикл { $to_db = preg_replace('@'.$id.',@', '', $db_sync->$prop['first']);// first - у нас содержит поле category в котором данные выглядят вот так: 2,66,9 $res_varchar = $db_sync->$prop['second']; //Сразу же заносим id строки в переменную т.к если будем еще работать и с удалением нам это понадобиться if (isset($prop['spec_type']['delete'])) //Проверяем есть ли массив с ключом delete если есть, значит надо что то удалять { $this->db->where($prop['secod_data'], $res_varchar); //second_data - содержит поле properties_id в базе myshop_properties_data таким образом id из myshop_properties == properties_id из myshop_properties_data $sql = $data_my[$prop['spec_type']['delete']]['dbs']; $this->db->delete($sql);//Если есть значения то удаляем } $data_sync = array($prop['first'] => $to_db);//Теперь опять работаем с update...в $to_db у нас содержится отредактированная информация без не нужных нам id $this->db->where($prop['second'], $res_varchar); $sql = $data_my[$prop['spec_type']['update']]['dbs']; if (strlen($to_db) > 1) //Если в $to_db символов больше чем 1 то обновляем, иначе просто удаляем строку это может быть если у нас было например в поле category 66, а нам как раз и нужно было исключить 66, из объекта { $this->db->update($sql, $data_sync); } else { $this->db->delete($sql); } } } } } } } } $this->template->add_array($data); $this->ajax_tpl('ajax'); } 
  • overloaded with comments - impossible to read - zb '
  • Sorry, I wanted the best)) pastebin.com/3FWZFhrm - no comments - Zeloras
  • do you know &&? I'm talking about this piece: if ($ prop ['spec_type_data'] === 'update_delete') {if (isset ($ prop ['spec_type'] ['update'])) { - zb
  • Of course! But if I understood everything correctly, you propose to do this: if ($ prop ['spec_type_data'] === 'update_delete' && isset ($ prop ['spec_type'] ['update'])) - did I understand correctly? if so then in this case you will have to use another cycle in the ['delete'] block - Zeloras
  • why these are two conditions nested in each other, without any intermediate operations and branching of the second condition. if(cond1) {if (cond2) {do()}}; How can changing to if (cond1 && cond2) {do()} affect how do () works? Not only that, I would change cond2 and cond1 in some places. - zb '

1 answer 1

 // TODO: decomposition is needed 

It is better to put the initialization of $ data_my into a separate method and neither initialize it every time, but initialize, save and, if it is requested again, return the saved one.

Make a method of checking the current action (instead of comparing in the code) and methods that perform various actions (delete, update, etc.)

 // TODO: no diving (или как там погружения называются?) 

Instead of diving into conditions, it is better to call return, continue or break, whichever is required.

Using your approach, you need to remember where-what-how it is and is indicated. Replacing everything with the appropriate methods you will already read the code (algorithm), and not the names of the parameters.

Removing the dive code will also make reading easier. Agree that the following is much easier to read:

 public function select_deleteAll($type) { $res_varchar = 0; $data = array(); // $data_my = $this->getDataMy(); - будем вызывать там, где это понадобится if ($this->dataMyIsSmall($type); //, $data_my)) { return false; } $this->activateMessageBlock($data, $type); $ids = $this->input->post('ids'); foreach ($ids as $id) { if ($id <= 0) { continue; } $this->deleteType($id); //, $data_my); $data['updated'] = true; if (!$this->isCycle($type)) { // $data_my)) { continue; } $propList = $this->getCycle($type); //$data_my); foreach ($propList as $prop) { $this->processPropAction($prop); //, $data_my); // В метод processPropAction вставить, например, следующее: //if ($this->isPropUpdate($prop)) //{ // $this->updateProp($prop); //, $data_my); // continue; //} //if ($this->isPropDelete($prop)) //{ // $this->deleteProp($prop); //, $data_my); // continue; //} //if ($this->isPropUpdateDelete($prop)) //{ // $this->updateDeleteProp($prop); //, $data_my); // continue; //} } } } $this->template->add_array($data); $this->ajax_tpl('ajax'); } 

I would have a little corrected here, but this is already better than yours.

  • Thank you, I was just thinking about how to make a separate model for this, can I still have a couple of questions? 1) For example, for pagination, I use $ count = $ this-db-> get ('table') -> num_rows () $ this-> db-> limit ($ on_page, $ page); $ this-> db-> get ('table') -> result (); The question in the next. Am I doing it right, or can I do something with one request? for example, get all the data at once and then output it from the array? - Zeloras
  • 2) For example, I make a gallery and on one of the specials. pages I need to display some categories, I use select multiple Then, after sending the data, I start a loop that tears out the value foreach ($ select_arr as $ sel) {if ($ sel! = '') {$ album_list. = $ sel. ','; }} and insert it into the database and for output I do this: $ albums = explode (',', $ albums_list); array_pop ($ albums); This is normal? or are there other better ways? Thanks in advance - Zeloras