приведу ниже код, хотел бы услышать всю критику, и советы, может , даже не может а я уверен что стоит что то дет поменять, для улучшения(т.к. я навичек в PHP фразы типа ты ламер, л..х, баран и т.д. я и без вас знаю =)) ) PHP: <?php ob_start(); Session_start(); ?> <html> <head> <!--подключение стилей--> <link rel="stylesheet" type="text/css" href="css/reg.css"> <link rel="stylesheet" href="css/validationEngine.jquery.css" type="text/css" media="screen" title="no title" charset="utf-8" /> <script src="js/jquery.js" type="text/javascript"></script> <script src="js/jquery.validationEngine.js" type="text/javascript"></script> <!--/подключение стилей--> <div id="main"> <!--Вывод форм для ввода данных--> <div id="form"> <form method="post" action="" class="formular"> <div id="forms"> <p class="class1">Login<sup class="sup"><small>*</small></sup>:</p> <input type="text" name="login" size="20" maxlength="100" class="validate[required,custom[onlyLetter],length[0,100]] text-input" id="login" value="<?=isset($_POST['login']) ? $_POST['login'] : '' ?>"><br> <p class="class2">E-mail<sup class="sup"><small>*</small></sup>:</p> <input type="text" name="email" size="20" maxlength="100" class="validate[required,custom[email]]" id="email" value="<?=isset($_POST['email']) ? $_POST['email'] : '' ?>"><br> <p class="class3">Пароль<sup class="sup"><small>*</small></sup>:</p> <input type="password" name="password" size="20" maxlength="30" class="validate[required,length[3,16],custom[onlyLetter]]" id="pass" value="<?=isset($_POST['password']) ? $_POST['password'] : '' ?>"><br> <p class="class4">Повторить пароль<sup class="sup"><small>*</small></sup>:</p> <input type="password" name="password2" size="20" maxlength="30" class="validate[required,ength[3,16],confirm[pass]]" id="pass2" value="<?=isset($_POST['password2']) ? $_POST['password2'] : '' ?>"><br> <p class="class5">Введите символы с картинки<sup class="sup"><small>*</small></sup>:</p><br> <!-- картинка защитного кода --> <div id=cap4a> <img src="secpic/secpic.php" alt="защитный код" class="cap4a"/> </div> <!-- /картинка защитного кода --> <input type="text" name="key" size="20" maxlength="100" value="" class="key"><br> <input type="submit" name="register" value="Регестрация" class="button"> <input type="button" name="cancel" value="Отмена" class="button" onclick="parent.location='../sign.php'"> </div> </form> <!--/Вывод форм для ввода данных--> </head> <div id="error"> <?php //проверка на нажитие кнопки регестрации if(isset($_POST['register'])){ //подключение к бд include "mysql/mysql.php"; //перенос данных из формы в переменную $login=$_POST['login']; $email=$_POST['email']; $password=$_POST['password']; $password2=$_POST['password2']; $key=$_POST['key']; //проверка на заполненые поля if (!empty($key)) { //проверка капчи if($_SESSION['secpic']==strtolower($key)) { $kod=1; } else { echo'<p class="error">-неверно введены цифры с картинки </p>'; } } else { echo'<p class="error">-символы с картинки не введенны </p>'; } //если символы с капчи введены верно тогда проверяются остольные поля //сначало заполнены ли они вообще а патом уже то что в них написанно if($kod==1) { //проверка логина на повторение в таблице(логин будет уникальным) if (empty($login)) { echo'<p class="error">-Login не введен</p>'; }else { $result=mysql_query("select login from active where login= '".$login."'"); if(mysql_num_rows($result)>0) { echo'<p class="error">-Извените данный логин уже используется</p>'; }else{$log=1;} } //проверка мыла if (!empty($email)) { //проверка мыла по шаблону if (!preg_match("/[0-9a-z_]+@[0-9a-z_^\.]+\.[a-z]{2,3}/i", $email)) { echo '<p class="error">-E-mail не введен или введен неверно</p>'; } else { //проверка мыла на повторение в базе(мыло является уникальным) $result=mysql_query("select mail from users where mail= '".$email."'"); $result2=mysql_query("select imeil from active where imeil= '".$email."'"); if(mysql_num_rows($result)>0 or mysql_num_rows($result2)>0) { echo'<p class="error">-Извените данный e-mail уже занят</p>'; }else{$m=1;} } }else{echo'<p class="error">-E-mail не введен</p>'; } //проверка пасса $pas=1; if (empty($password)) { echo'<p class="error">-password не введен</p>'; } else { //проверка на длинну пароля не менее 3-х не более 16-ти if(strlen($password)<3) { echo '<p class="error">-Длина пароля должна быть не менее 3-х символов !</>';$pas=0; } else { if(strlen($password)>16) { echo '<p class="error">-Длина пароля должна быть не более 16-х символов !</p>';$pas=0; } else { if($pas!=0) { //проверка на правельность ввода потверждающего пароля if ($password2!=$password) { echo'<p class="error">-Не совпадают пароли</p>'; } else{$p=1;} } } } } } //если все требования к заполнению форм выполнены то выполняется следующее if($kod==1&&$m==1&&$p==1&&$log==1) { //заносим данные во временную таблицу активации с уникальным кодом активации $dat=date("y.m.d"); $checkSum=md5(base64_encode(rand(55555,99999))); $query="insert into active (login,password,imeil,activekod,date) values('$login','$password','$email','$checkSum','$dat')"; mysql_query($query); //формировка и отправка письма $month=date("Y-m-d", mktime(0, 0, 0, date("m"), date("d")+7, date("Y"))); $silka="http://".$_SERVER['HTTP_HOST']."/city-chat/register/us2.php?checkSum=".$checkSum."&login=".$login.""; $texst="<p>Сегодня ".date("m.d.y")." в ".date("H:i")."на сайте City-Chat.lg.ua \n был зарегистрирован пользователь с вашим email'ом. Поэтому вы получили данное письмо. \n Если вы не регистрировались на нашем сайте, то попросто проигнорируйте данное письмо, а если \n же это были вы то перейдите по нижеприведённой ссылке.</p> <p>Активация аккаунт будет действителен до: ".$month.", после чего актевировать его будет невозможно будет невозможно !</p> <p>Ссылка для активации:\n <a href=\"".$_SERVER['HTTP_HOST']."/city-chat/register/us2.php?checkSum=".$checkSum."&login=".$login."\">$silka</a></p>\n __________________________________________________________________________________________________ <p>С уважением администрация City-Chat.com.ua\n Email для контактов:<a href=\"mailto: [email=toh1napocht@yandex.ru]toh1napocht@yandex.ru[/email]\"> [email=toh1napocht@yandex.ru]toh1napocht@yandex.ru[/email] </a></p> __________________________________________________________________________________________________ <p>Данное сообщение было созданно автоматически и отвечать на него не стоит!!</p> "; mail("$email", "Активация", "$texst" ,"Content-Type: text/html; charset=windows-1251","From: [email=city-chat@mail.ru]city-chat@mail.ru[/email]"); //перенаправляем скрипт header("Location: [url=http://]http://[/url]".$_SERVER['HTTP_HOST']."/city-chat/register/us2.php"); } mysql_close($db); } ob_end_flush(); ?> </div> <div id="info"> <div id="div-inf-login"> <span class="inf-login">-введите ваше уникальное имя</span> </div> <div id="div-inf-mail"> <span class="inf-email">-укажите ваш почтовый ящик на который будет отправленно письмо активации, вводите достоверные данные!</span> </div> <div id="div-inf-pass"> <span class="inf-pass">-введите ваш секретный пароль, который даст вам возможность входа на сайт, не забудьте его!</span> </div> <div id="div-inf-pass2"> <span class="inf-pass2">-повторите ваш пароль</span> </div> <div id="div-inf-cap4a"> <span class="inf-cap4a">-введите символы с картинки</span> </div> </div> </div> </div> </html>
для новичка неплохо. продолжай в том же духе, и скоро будешь чувствовать себя уверенно. критиковать и что-то улучшать можно всегда и все. главное помнить, лучшее - враг хорошего. я бы посоветовал обратить внимание на подобное PHP: $result=mysql_query("select mail from users where mail= '".$email."'"); здесь потенциальная дырка в безопасности, ибо параметры у тебя не фильруются. почитай на досуге об sql-инъекциях. и еще. не стоит пользователю конкретизировать, что у него неправильно введено имя пользователя или пароль. опять же из соображений безопасности. просто одним селектом аутентификация производится: [sql]select null from users where login=$email and passwd = $pass [/sql] ну это условно. если запрос вернул ноль строк, говорим юзеру - Фэйл, имя пользователя или пароль не верно и все.
навскидку 1. возможна sql injection при записи во временную таблицу активации, при проверке уникальности логина и вообще при любом запросе. 2. зачем нужна временная таблица? проще сделать в основной таблице users пару дополнительных полей - activation_code и is_active. 3. зачем отдельные запросы для проверки уникальности? Сделай эти поля уникальными в базе, пиши в нее запись и перехватывай sql exception. 4. зачем if($kod==1&&$m==1&&$p==1&&$log==1)? Вполне хватает одной переменной.
честно говоря пока что из слов про sql нифига мне не понятно(жаль, пропустил весь курс по sql ) про100 пока не читал, думаю через некоторое время пойму, "engager"- вообщем то там тока указывается что логин либо пароль введены или что пароль должен быть в диапазоне, но про пароль в диапазоне так же проверяет его jquery - ну типа проверка форм , "karakh"-смогу ответить ток на 4-й пункт, не знаю, мне так проще видно какое условие не выполняется а какое нормально прошло, то при отладке попробуй найди если все в одну переменную заносить
bookin для начала прикинь, как можно разделить html и php. Это сейчас у тебя хорошо, всего 200 с небольшим строк кода. До 1000 дойдешь - сам запутаешься.
PHP: <?php if ( /*...*/ ){ // } else{ if ( /*...*/ ){ // } else { if( /*...*/ ){ // } else{ if ( /*...*/ ){ // if ( /*...*/ ){ Не самое удачное решение PHP: <?php if ($kod==1&&$m==1&&$p==1&&$log==1) Именуйте переменные правильно
где-то так: PHP: <?php if (isset($_POST['login'])){ action_registration(); } else { block_show_form(); } die; define('USER_EMPTY_LOGIN', 1 ); define('USER_LOGIN_IS_SHORT', 2 ); define('USER_EMPTY_PWD', 3 ); define('USER_WRONG_CAPTCHA', 4 ); /** * Регистрация пользователя * * @return bool */ function action_registration(){ $errors = array(); validate_user_login($errors); validate_user_pwd($errors); validate_user_captcha($errors); if (count($errors)){ block_show_errors($errors); block_show_form(); } else { $user = create_user( $_POST['login'], $_POST['pwd'] ); send_user_message($user); header('Location: /'); die; } } /** * Проверить корректность логина * * @param $errors * @return bool */ function validate_user_login(&$errors){ if (empty($_POST['login'])){ $errors[] = USER_EMPTY_LOGIN; return false; } if (strlen($_POST['login']) < 3){ $errors[] = USER_LOGIN_IS_SHORT; return false; } return true; } /** * Проверить корректность пароля * * @param $errors * @return bool */ function validate_user_pwd(&$errors){ if (empty($_POST['pwd'])){ $errors[] = USER_EMPTY_PWD; return false; } return true; } /** * Проверить корректность каптчи * * @param $errors * @return bool */ function validate_user_captcha(&$errors){ // ... return true; }
на сколько я заметел вы по большому счету про100 все вынесли в функции, я приму ваш вариант к сведению, и еще вопрос вой не самый удачный вариант а в чем могут быть проблемы при таком варианте??
да, да, да, конечно, вы правы! сложность поддержки такого кода, это основная проблема. К примеру возьмите 93 строчку своего кода: Код (Text): if (!empty($email)){ Всё четко и понятно, "если не пуст email" (я так бы прочитал) Теперь возьмите 138 строчку и скажите: попадем ли мы сюда если email пуст? bookin, моя первая программа на php была точь в точь как ваша Я был счастлив и рад, думаю как и вы Но когда она выросла до 700-800 строк это был абзац. Я матерился на чём свет стоит: ничего не понятно, откуда что берется и куда что девается. Если изначальной целью было написать что-то хорошее, то под конец уже просто мечтал заставить это нагромождение работать, хоть как-то. Проверено практикой: 1. следует избегать уровня вложенности условного оператора больше двух 2. разбивайте свои программы на подпрограммы, выделяйте отдельные самодостаточные блоки 3. избегайте больших функций. 30 - 50 строк кода в одной функции это много. 4. читайте, Фаулера "Рефакторинг". Там есть необходимая вам информация.
Volt(220) +1, Макконнела тоже нужно читать В своей практике (так получается) 90% методов умещаются в 10-15 строк Когда количество кода в методе растёт я обычно выделяю функционал из метода в разные методы. Сигналом к выделению кода очень часто служит количество строк. Где-то строк 50 и я уже себя чувствую неуютно. Конечно, есть и другие сигналы, но это отдельная тема. Конечно, я писал функции и в 300 строк хотите в них разобраться? нет? Вот и я не хочу. А "сложные алгоритмы требующие написания именно в одной функции и именно 200 строк" - это больше похоже на бред, уж извините. Решающий фактор для меня это не ошибки, а скорость их устранения. Ошибки были есть и будут, а вот поиск и исправление ошибок это хороший аргумент, в пользу коротких методов, конечно. Volt(220), как вы считаете, что проще понять начинающему программисту: вот эта цитата или оговорка про 50 строк? UPD> Давайте отдельную тему, если желаете побеседуем
Да тут особо не о чем беседовать. =)) Я не оспариваю справедливость Ваших замечаний. Я просто не хочу чтобы люди принимали эти рекомендации как аксиомы. PS: строго говоря это не я писал. =))
те люди которые не совсем глупые и php это не первый язык думаю должны понять что сказанное не нужно принимать как факт =)
Даже у таких людей периодически какая-нибудь логичная фраза умного дядьки западает и начинает быть истиной и целью. Ну и потом, кто сказал что этот форум будут читать только "не совсем глупые" и у которых "php это не первый язык"?
Volt(220) никто этого не говорил, и прави льно что уточнили. у меня еще вопрос назрел)) engager сказал что не стоит конкретизировать какое поле не правильно заполнено из=за соображения безопасности, а в чем может быть угроза при таком варианте??
Подобрать два элемента сложнее чем один. Так, грубо говоря, если есть списки всех возможных (самых частых) логинов(например, x=1000 штук) и паролей(y=2000 штук), то для подбора пары логин-пароль в худшем случае нужно x*y=2 000 000 попыток. Если же система выдает информацию о том что неверно, то в худшем случае нужно x+y-1=2999 попыток.
именно. другими словами, что проще, подбирать пароль, зная имя пользователя, или подбирать и то и другое, не зная ничего?