Второ мнение по пропуски на код

P

phpbot

Registered
Имам нужда от второ мнение по този код, мисля че има дупка но мисля че съм ги оправил но все пак ако може да прегледа някой пич кода никога не е излишно второ мнение. :)
Давам +

Код: 
<?php
include("config.php");
$br = 6; 
$pageNum = 1; 
if(isset($_GET['pages'])) { 
$pageNum = addslashes(strip_tags(trim($_GET['pages'])));
$pageNum = ($pageNum)?$pageNum:1;
} 
$redove = ($pageNum - 1) * $br; 
$query = mysql_query("SELECT * FROM `posts` WHERE type='1' AND `kategoria`='$id' ORDER BY `id` DESC LIMIT $redove, $br");
$news = mysql_num_rows($query);
if($news == "0"){
echo "<center><b>Няма добавени новини.</b></center><hr />";
}else{
while($row = @mysql_fetch_array($query)){
$newid = $row['id'];
$newtitle = $row['title'];
$newdate = $row['date'];
$newcate = $row['cate'];
$newtitle1 = str_replace (" ", "_", $newtitle);
echo '<a href="/news/'.$newid.'-'.$newtitle1.'/">'.$newtitle.'</a><br /><hr />';
}
}
?>
</div>
      </font>
	</div>
	<div id="right-box"></div>
	<div id="right-box"></div>
	</div>
<?php
$cate = $id;
$cate = str_replace ("1", "homecars", $cate);
$cate = str_replace ("2", "cars", $cate);

$results = mysql_num_rows(mysql_query("SELECT * FROM `posts` WHERE `type`='1' AND `kategoria`='$id' ORDER BY `id`")); 
$pagination = new pagination(5, $results, $cate); 
$query = mysql_query("SELECT * FROM `posts` WHERE type=1 AND `kategoria`='$id' ORDER BY `id` DESC  LIMIT {$pagination->limit['first']}, {$pagination->limit['second']}") or die(mysql_error());
while($row = mysql_fetch_array($query)){
$newid = $row['id'];
$newtitle = $row['title'];
$newdate = $row['date'];
$newtitle1 = str_replace (" ", "_", $newtitle);
$newtext = nl2br($row['text']);


//the variable for the replace
$newtext = preg_replace("/\[b\](.*?)\[\/b\]/is", "<b>$1</b>", $newtext);
$newtext = preg_replace("/\[u\](.*?)\[\/u\]/is", "<u>$1</u>", $newtext);
$newtext = preg_replace("/\[i\](.*?)\[\/i\]/is", "<i>$1</i>", $newtext);
$newtext = preg_replace("/\[url\=(.+?)\](.+?)\[\/url\]/is", "<a href='$1' target='_blank'>$2</a>", $newtext);
$newtext = preg_replace("/\[url\](.+?)\[\/url\]/is", "<a href='$1' target='_blank'>$1</a>", $newtext);
$newtext = preg_replace("/\[url\](.+?)\[\/url\]/is", "<a href='$1' target='_blank'>$1</a>", $newtext);
$newtext = preg_replace("/\[youtube\](.+?)\[\/youtube\]/is", "<embed src='/flash3/player.swf' width='650' height='500' flashvars='&autostart=false&file=$1'></embed>", $newtext);
if(strlen($newtext) > 1120)
{
 $newtext = substr($newtext,0,1120)." .. <br/><br/> <a href='/новина/".$newid."-".$newtitle1."/' style='float:right;'>Дочети »</a>"; 
}else
	{
	 $newtext = $newtext;
	}


$numcomments = mysql_query("SELECT * FROM comments WHERE pid='$newid'");
$comments = mysql_num_rows($numcomments);
$cate = $row['kategoria'];
$cate = str_replace ("0", "no category", $cate);
$cate = str_replace ("1", "<a href=\"/homeCars\">HomeCars</a>", $cate);
$cate = str_replace ("2", "<a href=\"/cars\">Cars</a>", $cate);
$cate = str_replace ("3", "<a href=\"/Cars\">Cars</a>", $cate);

 
 
$img = $row['img'];
include 'date.php';
echo '<div id="news_content_preview">
<table>
<tr>
<td valign="top">
<h3 class="title">'.$newtitle.'</h3>';
echo '<div class="primary"><span class="meta_date">Публукувано на: '.$newdate.'</span>
  
 <span class="categories">Категория: '.$cate.', 
 </span>
  
 <span class="comments">'; 
if($comments == 0)
{
echo "<a href='/news/$newid-$newtitle1/'> <b>Няма коментари</b></a>";
}
elseif($comments == 1)
{
echo "<a href='/news/$newid-$newtitle1/'> <b>$comments</b> коментар</a>";
}
else
{
echo "<a href='/news/$newid-$newtitle1/'> <b>$comments</b> коментара</a>";
}
echo '';
}
echo "<div class=\"news_title_preview\"><center>".$pagination->output."</div></center>";
?>
 
PHP: 
$pageNum = addslashes(strip_tags(trim($_GET['pages'])));

Това е излишно, като можеш просто да му кастнеш типа направо

PHP: 
$pageNum = (int)$_GET['pages'];
 
И това, ако е перфектно писане на код.


1. @Radko ти го каза
2. mysql_num_rows() връща стойност от тип int, а не string, защо проверяваш $news == "0" ?
3. Защо потискаш грешки с @ ? Направи съответната проверка и извеждай съответното съобщение.
4. Защо си оспагетяваш(от спагети) кода, като допълнително присвояваш на нови променливи стойностите от $row( $newdate = $row['date'] ) . Използвай ги директно.
5. Това $cate = $id; от къде идва въобще не се знае. Говоря за $id.
6. Имаш много излишни заявки. Направи една заявка, сложи нужните неща в един масив и след това върти масива, както и където ти трябва. Пращаш ненужни заявки към сървъра.
7. Твърде много хардкодваш. Отделяй малко визуализацията с логиката.
8. На места ползваш едни кавички, на други места други... Или използвай само едните, или само другите.


Това е. За дупки не мога да ти кажа, защото е достатъчно объркан кода. Заявките даже не съм ги чел, но вероятно и там ще има "недодялани" неща.
 
Е да кажа че кода не е мой :) аз правя защита не съм ви карал поне тебе Tryme да оценяваш подредбата, но все пак благодаря за забележките били дал твой сайт на листа :)


Ако може и други мнения по кода дали има пропуски :) приятели.
 
phpbot каза:
били дал твой сайт на листа :)


Ако може и други мнения по кода дали има пропуски :) приятели.
Увеличете...


Това не го разбрах.


Ето други мнения.
Все пак не разбрахме от къде идва това $id. Дефинирано е предварително или идва някъде отвън($_POST, $_GET), защото все пак влиза в заявка в WHERE клаузата. Как да ти кажа дали няма да иска някаква добавка, като не знам от къде идва ?

Нямам други забележки.


П.П. Впрочем, предполагам, че $id = $_GET['id']; (поправи ме ако греша). Та, ако категориите ти започват от 1, то направи каст и направи пак същата проверка като $pageNum = ($pageNum)?$pageNum:1; или направи така, че ако върне 0, то да казва, че няма такава категория.
 
2 неща

1.Напиши целия код отново.
2.Не знам ако ти възниква проблем с кода с тея @ как ще разбереш от каде е.

Ама аз съм аматьор нямам право на мнение :P :)
 
Трябва да си влезнал или регистриран, за да отговориш тук.

Back
Горе