Оцените код

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

// Вывод всех гороскопов

					if ( $_GET == 1 )

					{

						// Получение текущей даты для вывода гороскопа по сегоднящнему дню

						$today_date = date(md);


						//Если введен номер гороскопа

						if ($_GET !="")

						{



							// Получаем данные

							$znak = $_GET;


							echo "<tr valign=top><td>";

							echo "<table>

								<tr>

									<td>

											<img src=pictures/1.jpg align=top>

										</td>

										<td>";


										//Выборка данных для вывода гороскопа

										$query = "SELECT text_horo FROM every_day WHERE znak='$znak' AND date_start='$today_date'";

										$result = mysql_query ( $query );

										@$res = mysql_fetch_array ( $result );


											//Вывод данных

											echo $res;


											echo "

										</td>";

								echo "</tr></table>";

							echo "<center>";

								echo "<a href=index.php?module=1>Назад</a>";

							echo "</center>";

							echo "</td></tr>";

						}

						else

						{


							//Вывод гороскопов всех знаков

							$j=1;

							for($i = 0; $i<=6; $i++)

							{

								echo "<tr valign=top>";


								for($k =1; $k<=2; $k++)

								{


									//Выборка данных для вывода гороскопа

									$query = "SELECT text_horo FROM every_day WHERE znak='$j' AND date_start='$today_date'";

									$result = mysql_query ( $query );

									@$res = mysql_fetch_array ( $result );


									echo "<td width=10%>";

										echo $res;

									echo "</td>

										<td>";


										echo $res;

										echo "<br>";

										echo "<a href=index.php?znak=$j&module=1>Подробнее</a>";

									echo "</td>";


									$j++;

								}


								echo "</tr>";

								echo "<tr>";

									echo "<td>";

										echo "<hr width=100%>";

									echo "</td>";

									echo "<td>";

										echo "<hr width=100%>";

									echo "</td>";

									echo "<td>";

										echo "<hr width=100%>";

									echo "</td>";

									echo "<td>";

										echo "<hr width=100%>";

									echo "</td>";

								echo "</tr>";

							}

								echo "<tr>";

									echo "<td>";

										echo "<hr width=100%>";

									echo "</td>";

									echo "<td>";

										echo "<hr width=100%>";

									echo "</td>";

									echo "<td>";

										echo "<hr width=100%>";

									echo "</td>";

									echo "<td>";

										echo "<hr width=100%>";

									echo "</td>";

								echo "</tr>";

						}

					}

					else

					if ( $_GET == 2 )

					{

						// Фэншуй

						if ($_GET == 1)

						{

							//Вывод статей по фэншую

							$query = "SELECT text_fen,id FROM fenshuj WHERE id=".$_GET."";

							$result = mysql_query ( $query );

							$res = mysql_fetch_array ( $result );

							echo $res;

							echo "<hr width=100%>";

							echo "<br>";

							echo "<center><a href=index.php?module=2>Назад</a></center>";

						}

						else

						{

							// Вывод ознакомительного текста

							$query = "SELECT text_fen,id FROM fenshuj WHERE title_fen='main'";

							$result = mysql_query ( $query );

							$res = mysql_fetch_array ( $result );

							echo $res;

						}				

					}

					e

Написано ужасно.

Вкратце:

1) мешанина хтмл-кода и логики
2) отсутствие защиты от SQL-иньекций (потенциальная дырка, а не код).

Что-то сначала все ошибки решил расписать, разъяснения всякие.
Потом смотрю на , , ; "blabla '$k'", "blabla ".$k.""; (bla), ( bla )
2jamper, займись лучше пением.

$znak = $_GET;

Нет проверки на тип и содержимое (вообще знаков в гороскопе 12, вот и делайте цифрой + проверку вставьте, что это именно цифра).

echo "</tr>";

echo "<tr>";

echo "<td>";

echo "<hr width=100%>";

echo "</td>";

echo "<td>";

echo "<hr width=100%>";

echo "</td>";

echo "<td>";

echo "<hr width=100%>";

echo "</td>";

echo "<td>";

echo "<hr width=100%>";

echo "</td>";

echo "</tr>";

Подобное вообще надо исключать. Отдельно функции php, отдельно код html.

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

Цитата ("jamper"):

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

jamper, ответьте пожалуйста на следующие вопросы:
1. Скажите Ваше мнение, что в этом Вашем коде хорошего, и что плохого. Составьте список, по пунктам.
2. Пожалуйста напишите, чем Вы руководствовались когда "собсно оформляли код"?
3. После этого задумайтесь, а нужно ли было выставлять свой код, чтобы другие сказали Вам то же самое, что Вы и так про свой код знаете?

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

tvv, да я уже понял что несколько погарячился
Собсно буду редактировать код и покажу что и как после выйдет

Цитата:

@$res = mysql_fetch_array ( $result );

как мило...

Кстати, господа оценивающие... Топикстартер написал:
Цитата:

В принципе делаю только первые шаги в изучении php и соответственно программировании на нем.

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

Первые шаги - это когда человек делает что-то в первый раз.
А тут copy/paste и немного глупости.

Что вы на человека наехали, ну начинает он! так объясните ему.
ИМХО, ставь табуляции на новых ступеньках логики, иначе или запутаешся сразу, или когда через 1 месяц надо будет проект переделать!

if (условие)

{

->     тело_функции

->     if (Условие2)

->     {

->->            тело_функции2

->     }

}

"->" это типо табуляция, по другому не знаю как сделать! Надебсь понятно!

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

Дели PHP и HTML код, не делай HTML через echo. Закрыл PHP спокойно пишеш на HTML. В крайнем случае лучше напиши <?$переменная?> прямо в HTML коде!


Остальное даже не дочитал! Но вобще от "@" это игнорирование ошибки, а там её быть не может! Он в любом случае возьмёт массив, просто массив может оказаться пустым. Хотя мне кажеться что в данном контексте не принципиальная ошибка.


Удачи!

jamper, Написано ужасно

Цитата ("ArtLab"):

2) отсутствие защиты от SQL-иньекций (потенциальная дырка, а не код).

Согласен.

jamper, чтобы обезопасить свой скрипт, фильтруй, проверяй данные.
Почитай вот это:
ссылка
ссылка
ссылка
ссылка

ТЩАТЕЛЬНО фильтруй, проверяй данные GET, так как их очень легко подделать.

И ещё:
@$res = mysql_fetch_array ( $result ); - Это не правильно
$res=@mysql_fetch_array($result); - Вот это правильный вариант

Удачи !

jamper, выучи нормально синтаксис, oop, шаблоны проектирования.
Потом учи какой-нить фрэймворк, limb-project.com мне очень нравится...
И пиши всё на нём... Остальное придёт...

Я считаю это самый быстрый способ научиться кодить качественно и быстро.
Конечно же это моё IMHO, я могу ошибаться.

Спасибо всем, кто поделился нормальными советами. Сказать иди лучше дворником работать, я тоже могу! Не у всех же все получается сразу!!! А те кто дал дельные советы, то я их выслушал и принял во внимание.

Спс.