Fredrich
Стиль ужасный, подправь и выложи ещё раз уже с нормальным стилем.
Вот требования: http://irbis-team.com/15/1/8 , примеры хорошего стиля можешь посмотреть на конкурсе, там в первых работах показано.
Действия внутри цикла выделяются на +4 пробела.
echo - это не функция, поэтому скобки не нужны.
не надо выводить на белом экране echo, тебе надо отправить в переменную данные и вывести именно в нужном месте на странице эту переменную.
Пароли нельзя хранить без md5.
htmlspecialchars для логина нельзя применять, иначе логин будет существенно отличаться от того, который пожелал автор, что в своём роде неправильно.

Половину ошибок разглянуто у меня в подписи по ссылкам, глянь особенно на последнюю.

<?php
if (isset($_SESSION['id']))
    header("Location: index.php"); exit;
	
if (isset($_POST['login']) && isset($_POST['password'])) { 
    $login    = trim($_POST['login']);
    $password = trim($_POST['password']);
        
    $query    = "SELECT   `id`
                 FROM     `users` 
                 WHERE    `login`    = '".mysql_real_escape_string($login)."' 
                 AND      `password` = '".mysql_real_escape_string($password)."'";
      
    $select_user   = mysql_query($query) or die(mysql_error());
    $num_user      = mysql_num_rows($select_user);
  
    if ($num_user > 0) {
        $result_user = mysql_fetch_assoc($select_user);
        $_SESSION['id'] = (int)$result_user['id'];
        header("Location: index.php");
    exit;
    } else {
        die('Логин или пароль введен не верно!');
    }
} else {
    die('Логин или пароль не введен!');
}
?>

По мне так красивей и лучше.

alex12060, зачем два и более пробелов между переменной и равно?
Лишние фигурные скобки.
Каждый новый оператор писать лучше на новой строке (первый exit), на одном горизонтальном отступе (второй exit)
В запросе AND выглядит как самостоятельная команда, а не часть WHERE.
Для одной строки нет смысла использовать mysql_num_rows.

В целом код выглядит как пример или небольшая библиотека.
Настоящий код так хрен выровняешь - нервов не хватит на пробел столько жать.

neadekvat

1. При объявлении свойств это, может, нормально смотрится. Но в текущем коде не стоит доходить до фанатизма, имхо.

2.

else {
        die('Логин или пароль введен не верно!');
    }

=>

else
    die(...);

4. Ничего. Делаешь mysql_fetch_assoc и проверяешь вернувшийся массив.

Конечно, филосовский вывод, но по-моему, лучше проверить, есть ли что из запроса, а потом собирать массив, а не проверять массив.

с num_rows() жить легче, но она увеличивает код.

Хм, я думал не прокатит без скобок на else , но реально работает. Ух, форум форум, опыт бесценный.

neadekvat
Ну что ж, критика на критику критики, то есть тебя критикуем :)
Ну для начала, мог бы всю свою критику на конкурс отправить, она там очень нуждается в тебе :)

В запросе AND выглядит как самостоятельная команда, а не часть WHERE.
Я всегда пишу:

WHERE `
  AND `
  AND `
  AND `

То есть 2 пробела от WHERE, тогда видно, что она и внутри WHERE, а не как отдельный, но и с новой строчки.

Для одной строки нет смысла использовать mysql_num_rows.
Если таблица пустая, а ты сразу mysql_fetch_assoc сделаешь, то получешь error или warning, по крайней мере у меня постоянно апач возвращает, поэтому я использую mysql_num_rows.

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


А теперь критикую критикующего:
Если $num_user = 0, это false, если не 0, значит true, поэтому достаточно писать:
if($num_user) , без знаков > 0.
В добавок, так лучше вообще без лишней переменной:
if(mysql_num_rows($select_user)) {

$_SESSION['id'] = (int)$result_user['id'];
- тут (int) лишнее, так как мускул не пропустит не INT в ячейку integer в БД.

if (isset($_SESSION['id'])) header("Location: index.php"); exit; - всегда вернёт exit, потому что ты написал бурду:

if (isset($_SESSION['id'])) {
    header("Location: index.php");
}
exit;

Ну и последнее, isset($_POST,$_POST) - можно объединять.

Ну а что можно с безопасностью сделать?

Я б вот это чучуть переписал б

$num_user      = mysql_num_rows($select_user);
  
    if ($num_user > 0) {
        $result_user = mysql_fetch_assoc($select_user);
        $_SESSION['id'] = (int)$result_user['id'];// Такое насильственно привидение типов для CИ полезно
                                                                   // тут и без него все норм будет
        header("Location: index.php");
    exit;
    } else {
        die('Логин или пароль введен не верно!');
    }
} else {
    die('Логин или пароль не введен!');
}

на

    if (mysql_num_rows($select_user)) {
        $result_user = mysql_fetch_assoc($select_user);
        $_SESSION['id'] = $result_user['id'];
        header("Location: index.php");
    exit;
    } else 
        die('Логин или пароль введен не верно!');
} else 
    die('Логин или пароль не введен!');

Кста, что сессию-то не стартуешь? Или подключаешь файл куда-то?

neadekvat
Я её уже приводил ранее на форуме, под рукой нет кода, где её могу показать, так что код не покажу.
Я знаю эксперта, который никогда сокращенно без скобок не использует, и это его право, зачем навязывать свою мысль, если ему так удобнее?

neadekvat
Ты слишком категоричен в этом вопросе, хотя вопрос не относится к программированию, а к вопросу удобства.

Ребята Вы отклонились от сути вопроса!!! Вопрос был не о качестве внешнего вида кода, а о его работоспособности. Так как я делаю проект первый раз и для себя о красоте кода не может быть и речи.

Fredrich, во-первых, речь не только может быть, а должна быть. Во-вторых, в первом же ответе тебе предложили ссылку, по которой можно выровнять нормально код. От тебя мы так его и не увидели - поэтому обсуждаем другие темы.
Невозможно, пойми, что-то сказать о коде, который непонятно написан.

Fredrich Я бы наверное так сделал бы:

if($_SESSION['id']) {
	header("Location: index.php");
	exit();
}

if($_POST['login'] && $_POST['password']) { 
	function safesql($source) {
		$source = trim($source);
		if(get_magic_quotes_gpc()) $source = stripslashes($source);
		return htmlspecialchars(mysql_real_escape_string($source));
	}
	
	$query = mysql_query("
		SELECT `id`
		FROM `users`
		WHERE `login` = '".safesql($_POST['login'])."' AND `password` = '".safesql($_POST['password'])."'
		LIMIT 1
	") or die(mysql_eror());
	
	if(mysql_num_rows($query) > 0) {
		
		$data = mysql_fetch_assoc($query);
		$_SESSION['id'] = (int)$data['id'];
		
		header("Location: index.php");
		exit();
		
	} else {
		exit('Логин или пароль не введен!');
	}
}

Игорь_Vasinsky да ) Я тоже заметил, скопировал код автора

я имел ввиду в главном индексе распределить по блоком переклячения модулей

switch -> case, на irbis-team.ru есть пример, не примере паттерна mvc

Я по нему и делаю но там только 2 модуля в котором подключаеться только один контроллер, а у меня более сложная структура

а для чего? чтоб скучно не было?

покажи схематично как переключаешь модули, что подключаешь, как подключаешь

первый модуль отвечает за работу с пользователями которые не зарегистрированные
Второй работает с зарегистрироваными
Третий Это администраторский модуль


Я хочу сделать таким способом

1 модуль будет подключен по параметру page=main

и тут проводит пользовалетель манипуляции по средству нажатия ссылок с параметром action = (к примеру) FAQ и ссылка будет иметь вид

index.php?page=main&action=faq

и так все остальные

Где то вот так

да конечно есть проверка на существование сесии она и являеться переключателем с модуля работы с не авториризированого пользователя на залогиного, а что с сылками не так?

Fredrich
Для пароля: md5

Velbox
Не надо ни пароль, ни логин обрабатывать htmlspecialchars, потому что входящие данные надо либо на уровне формы фиксировать, либо заносить их именно такими, какие сделал автор. Уж тем более для пароля, так как его всё равно через md5 прогоняют.

а насчет структуризации что скажешь, см. выше

Fredrich
я написал всё на первой странице. Когда ПОЛНОСТЬЮ всё исправишь и перепишешь, тогда можно говорить дальше.

inpost согласен с тобой, что надо использовать вариант автора при занесении данных в БД, но если мы вытаскиваем определенную запись, надо обезопасить через htmlspecialchars, ведь данные можно подставить при выборке.

velbox
ну это уже при выводе: echo htmlspecialchars($row['login']);