Проблема с выбранной подкатегорией при редактировании элементов категории

Хорошо, в прошлый раз, когда я пытался помочь вам, вы набросились на меня, и я не знаю, почему я помогаю вам на этот раз, но вот некоторые улучшения в вашем коде , включая желаемую функцию.

В принципе, ваш код является грязной и трудночитаемой смесью HTML и PHP, которая слишком много делает в одной функции. Если что-то пойдет не так, вы не знаете, где. Мы разложим его на более мелкие многоразовые куски. Это увеличит ремонтопригодность, удобочитаемость и безопасность.

Вот как будет выглядеть код в конце:

 <?php error_reporting(-1); // turn on all errors function getCategoryFormEdit($id) { $categories = getCategories(); $category = getCategoryFromRowsById($categories, $id); $selectBox = createCategorySelectBoxOptions($categories, $category['subcat']); $html = getRenderedTemplate('categoryFormEdit.tpl.html', array( '{{category:name}}', '{{category:description}}', '{{category:selectBoxOptions}}', ), array( $category['name'], $category['description'], $selectBox )); return $html; } 

Как вы можете видеть, я сломал код, и теперь легко понять, что он делает от имен функций. В принципе, все getCategoryFormEdit делает сейчас, получает все необходимое для сборки HTML для вас. Оставшаяся часть выполняется в других функциях. Это делает код намного легче обрабатывать, потому что теперь он делает только одно, а не четыре. Считается хорошей практикой держать функции маленькими и позволять им делать одно за раз, потому что, если что-то пойдет не так, вам не нужно искать несколько сотен строк кода, но только в своей небольшой функции.

Другие функции заключаются в следующем:

 function queryDatabase($sql) { $result = mysql_query($sql); $rows = mysql_fetch_array($result); return $rows; } 

Поскольку вы будете выполнять запрос и выборку несколько раз, мы реорганизуем код для вызова базы данных в отдельную функцию. Мы ленивы и не хотим снова и снова воспроизводить эти звонки в нашем коде. Вместо этого мы просто передаем эту функцию некоторому SQL и возвращаем строки из запроса с запросами:

 function getCategories() { return queryDatabase('SELECT id, name, description, subcat FROM category'); } 

У вас был запрос с * в вашей версии. Считается хорошей практикой только извлечь из базы данных то, что вам действительно нужно от нее, поэтому я заменил ее именами столбцов, найденными в HTML. Запросы базы данных дороги, и чем меньше вы запрашиваете базу данных, тем лучше. Второй:

 function getCategoryById($id) { $sql = sprintf( 'SELECT name, description, subcat FROM category WHERE id = %d', $id); return queryDatabase($sql); } 

Обратите внимание, что я не включил идентификатор непосредственно в запрос, но с помощью sprintf и %d . В вашем коде вы использовали $ID = $_GET['id'] а затем вставили $ID прямо в SQL String, открыв свой код для SQL Injection . Никогда этого не делай. Вы не можете доверять пользователю и всегда должны его дезинфицировать . С %d мы удостоверяемся, что ID – это число и только число.

Если вы снова посмотрите на getCategoryFormEdit , вы заметите, что мы не вызываем getCategoryById($id) хотя и выполняем следующую функцию:

 function getCategoryFromRowsById($rows, $id) { foreach($rows as $row) { if($row['id'] === $id) { return $row; } } return FALSE; } 

Причина этого проста: при выполнении getCategories() вы уже выбрали все категории из базы данных, поэтому нет причин повторно запрашивать базу данных, чтобы получить только одну конкретную категорию, поскольку она включена в строки, возвращаемые во всех категориях , Помните, что запросы db дороги, поэтому давайте просто перебираем по всем категориям и выбираем строку с заданным $ id оттуда.

После того как вы получили все категории, вы можете использовать их для создания опций выбора:

 function createCategorySelectBoxOptions($rows, $selectedId) { $options = ''; foreach($rows as $row) { $options .= sprintf( '<option value="%s"%s>%s</option>', $rows['id'], ($rows['id'] === $selectedId) ? ' selected="selected"' : '', $rows['name'] ); } return $options; } 

Опять же, вся эта функция – это одно : она создает варианты выбора. Больше ничего. Он также добавляет selected атрибут к опции, которая имеет $row['id'] идентичную указанному $selectedId . С помощью sprintf снова вместо конкатенации строк, потому что его гораздо легче читать и добавляет в ваш код меньше помех. Также обратите внимание, что я использовал нотацию XHTML для выбранного атрибута. Вы можете сделать это только при использовании HTML-вкуса.

Наконец, весь материал, который мы только что подготовили, должен войти в шаблон. Для этого вы просто создаете файл шаблона и добавляете к нему код с заполнителями:

 <div class="form"> <h2>Add new category</h2> <form method="post" action="?page=editCategory"> <ul> <li><label>Kategori</label></li> <li><input type="text" class="inp" name="cname" value="{{category:name}}"></li> <li><label> Aç&#305;klama</label></li> <li><textarea class="inx" rows="10" cols="40" name="kabst">{{category:description}}</textarea></li> <li> <input type="checkbox" class="ins" name="kselect" id="kselect"> <label for="kselect">Aktif/Pasif</label> </li> <li>Üst kategorisi</li> <li> <select class="ins" name="subsl"> {{category:selectBoxOptions}} </select> </li> <li>Home page:</li> <li> <input type="checkbox" class="ins" name="kradio" id="kradio"> <label for="kradio">Aktif/Pasif</label> </li> <li><input type="submit" class="int" value="ekle" name="ksubmit"></li> </ul> </form> </div> 

Причина, по которой мы используем шаблон вместо написания всего HTML в этой функции, состоит в том, что мы хотим разделить презентацию и логику. Возможно, вы работаете с дизайнером, который не знает PHP, но может делать HTML и таким образом, вы можете легко разделить работу. Также легко просто изменить шаблон без необходимости поиска в вашем PHP-коде, где вы его разместили.

Далее нам нужен способ замены заполнителей, для чего используется следующий метод:

 function getRenderedTemplate($template, $placeholder, $replacements) { return str_replace($placeholder, $replacements, file_get_contents($template)); } 

Это чрезвычайно упрощенный подход к шаблонам. Вы также можете использовать DOM для шаблонов или выделенный движок шаблонов, например Twig или что-то еще. В этом примере я просто хотел, чтобы это было простым, поэтому я использовал str_replace для замены всех заполнителей соответствующими значениями и возврата заполненного шаблона к любой функции, называемой им, например

 if(is_numeric($_GET['id'])) { echo getCategoryFormEdit($_GET['id']); } else { die('Category ID must be numeric. Exiting'); } 

Вот и все. Рефакторинг сделан. Я не тестировал вышеприведенный код, поэтому в нем, вероятно, есть некоторые ошибки, но в любом случае: главное – показать, как улучшить код, как показано в вопросе. В принципе, всякий раз, когда ваши функции очень долго смотрят, попробуйте описать код в словах. Для каждого «и» рефакторируйте код на меньшую часть, которая просто делает одно. Сделав это, мы получили:

• функция повторного использования запросов, возвращающая строки из SQL
• функция многократного использования, которая получает все категории из БД
• функция повторного использования, которая получает категорию по идентификатору
• функция многократного использования, которая создает поле выбора из категорий
• многоразовый шаблон для мини-шаблонов
• разделение презентации и бизнес-логики
• фиксированная очевидная уязвимость SQL Injection
• лучшей читабельности и ремонтопригодности.

Если вы вернетесь к коду через несколько недель или месяцев, вам будет гораздо труднее выяснить, что он делает.

Я решил проблему:

 $selected = $d == $subcat ? " selected " : ""; $text .= '<option value="'.$d.'" '.$selected.' >'.$r['name'].'</option>'; function sQuery($x, $y) { $selected = $x == $y ? " selected " : ""; return $selected; } 

Вам нужно использовать ключевое слово global внутри функции:

  function sQuery() { global $r['id']; global $subcat; if ($r['id'] == $subcat) { $t = "selected"; } else { $t = ""; } return $t; }