Привет мастера. У меня вопрос по повышению мастерства в написании кода php. В моём коде всё работает, но хотелось бы услышать критику, и советы по улучшению "читабельности кода" и оптимизированной работы скрипта. У меня есть форма. И тут я проверяю, пустые ли поля (данные не фильтруются, это пока только начальная стадия написания). Подскажите, как лучше сделать. Как я, или, знаю, что можно сделать через switch? PHP: echo '<div class="activ"> <form method="POST" action="create_work.php" enctype="multipart/form-data"> Название работы:<br> <input type="text" name="work_name"><br><br> Описание:<br> <textarea name="opis" cols="40" rows="4"></textarea><br><br> Цена:<br> <input type="number" name="price"><br><br> Категория:'; $q = $db -> query("SELECT * FROM `cats`"); echo ' <select name="cat">'; while ($row = $q -> fetch_array()) { echo '<option value="'.$row['id'].'">'.$row['name'].'</option>'; } echo '</select><br><br> Скрин: <input type="file" name="img"><br><br> <input type="submit" value="Создать категорию"> </form> </div>'; if (isset($_POST['work_name'], $_POST['opis'], $_POST['price'], $_POST['cat'], $_FILES['img'])) { if (empty($_POST['work_name'])) echo 'Вы не ввели имя работы<br>'; if (empty($_POST['opis'])) echo 'Вы не ввели описание работы<br>'; if (empty($_POST['price'])) echo 'Вы не ввели цену работы<br>'; if (empty($_POST['cat'])) echo 'Вы не выбрали категорию<br>'; if (!is_uploaded_file($_FILES['img']['tmp_name'])) echo 'Вы не выбрали файл для добавления'; if (!empty($_POST['work_name']) && !empty($_POST['opis']) && !empty($_POST['price']) && !empty($_POST['cat']) && is_uploaded_file($_FILES['img']['tmp_name'])) { $uploaddir = '../works/img/'; move_uploaded_file($_FILES['img']['tmp_name'], $uploaddir.$_FILES['img']['name']); $filename = $_FILES['img']['name']; $q = $db -> query("INSERT INTO `works`(cat, name, screen, opis, price) VALUES ('".$_POST['cat']."', '".$_POST['work_name']."', '".$filename."', '".$_POST['opis']."', '".$_POST['price']."')"); if (!$q) echo $db -> error; echo 'файл добавлен'; } }
Во-первых, советую сменить подпись, мой вариант лучше как мне кажется) Пытаюсь говнокодить на PHP. Возможно вы говорите с будущим фрилансером-говнокодером, работающим за 30-40 тыщ в месяц Во-вторых, хочешь ревью кода? Ну получи, распишись... 1. Золотое правило - сначала код на PHP, затем разметка. У тебя все наоборот, при этом почему-то разметка обернута в длиииииную инструкцию echo. Зачем? Почему нельзя вывести непосредственно? 2. Зачем использовать $q->fetch_array(), если ты используешь только ассоциативные массивы? Используй $q->fetch_assoc() 3. Вместо громоздкой и неудобной конструкции PHP: if (isset($_POST['work_name'], $_POST['opis'], $_POST['price'], $_POST['cat'], $_FILES['img'])) { нужно проверять на то, что текущий метод доступа - POST и размер суперглобального массива POST больше 0 PHP: if ($_SERVER['REQUEST_METHOD'] == 'POST' && count($_POST) > 0) { // ... } 4. Ошибки валидации не нужно вываливать портянкой с помощью echo. Собери все сообщения об ошибках в массив, а потом выведи их с помощью foreach в одном месте, выделив красным Т.е. вместо PHP: if (empty($_POST['work_name'])) echo 'Вы не ввели имя работы<br>'; if (empty($_POST['opis'])) echo 'Вы не ввели описание работы<br>'; if (empty($_POST['price'])) echo 'Вы не ввели цену работы<br>'; if (empty($_POST['cat'])) echo 'Вы не выбрали категорию<br>'; Нужно что-то вроде PHP: $errors = []; //PHP >= 5.4 if (empty($_POST['work_name'])) { $errors[] = 'Вы не ввели имя работы'; } if (empty($_POST['opis'])) { $errors[] = 'Вы не ввели описание работы'; } ... а затем, где-то в глубинах HTML вывести массив $errors с помощью короткого синтаксиса PHP: <? if (is_array($errors) && count($errors) > 0): ?> <ul> <? foreach ($errors as $error): ?> <li><?= $error ?></li> <? endforeach; ?> </ul> <? endif; ?> 5. PHP: $q = $db -> query("INSERT INTO `works`(cat, name, screen, opis, price) VALUES ('".$_POST['cat']."', '".$_POST['work_name']."', '".$filename."', '".$_POST['opis']."', '".$_POST['price']."')"); Где проверка на безопасность? Где mysql_real_escape_string() ? Оставишь код вот так без экранирования и тебе "насуют за обе щеки" (с) 6. Скорее всего еще есть уязвимости в загрузке файла: нет проверки на тип, небезопасно обрабатывается путь и т.д. Но это ты уже найдешь и исправишь сам
@mahmuzar, ну раз не согласен - обоснуй в чем я не прав и расскажи как надо действовать в таком случае.