Is this function OK?

Hello,

All my website functions are like the one i will paste below.
i wanted to ask,
has this function any possible bug ? will this function make the server load hard ? can you suggest anything better for my website ? there are a lot of pageviews at my web, so i need to get my data without loading hard my server.

the code ( this is used to get the single posts.)

function single($id = '') {
	$id = mysql_real_escape_string ($id);
	$sql = 'SELECT id,post_title,post_date,post_content FROM wp_posts WHERE id='.$_GET['id'].'  LIMIT 1';
	$res = mysql_query($sql) or die (mysql_error());	

if (mysql_num_rows($res) !=0):
	while ($row = mysql_fetch_assoc($res)) {
	
	//this filter the content from the database
	$mycontent = $row['post_content'];
	$mycontent = strip_tags($mycontent);
	$mycontent = preg_replace("/\\[caption.*\\[\\/caption\\]/", '', $mycontent); 
	$mycontent = htmlentities($mycontent);
	
	//this make possible  to show special characters on title
	$title = $row['post_title'];
	$title = htmlentities($title);
	
	//date format
        $old_date = $row['post_date'];            
	$old_date_timestamp = strtotime($old_date);
	$new_date = date('d.m.Y   H:i', $old_date_timestamp); 
	
	//get first post image
	$first_img = '';
	ob_start();
	ob_end_clean();
	$my1content = $row['post_content'];
	$output = preg_match_all('/<img.+src=[\\'"]([^\\'"]+)[\\'"].*>/i', $my1content, $matches); 
	$first_img = $matches [1] [0];
	if(empty($first_img)){ //Defines a default image
    $first_img = "/img/default.png";
	}
	
	echo '
		<div class="single-header">
		<div class="single-title">'.$title.'</div>	
		<div class="single-tr"> '.$new_date.'</div> 
		</div><!-- single header -->
		<div class="single-print"></div><!-- print -->
		<div class="single-content">
		<div class="single-img">
		<img src="timthumb.php?src='.$first_img.'&amp;h=223&amp;w=395&amp;zc=1" alt="" />
		</div>
		<div class="single-text">'.$mycontent.' </div>
		</div> <!-- content -->
	'; //echo
}
	else:
		echo 'Dont exist';
	endif;
} // end 

This is very important for me , please check it , any kind of help will be just great

Thank you a lot for reading this thread.

There are several things I don’t get / would do differently


function single($id = '') {
	$id = mysql_real_escape_string ($id);
	$sql = 'SELECT id,post_title,post_date,post_content FROM wp_posts WHERE id='.$_GET['id'].'  LIMIT 1';

Why do you have an sanitize a parameter $id, and then don’t use it, but use $_GET[‘id’] instead? That defeats the whole purpose of having the $id in the first place. Ditching $_GET[‘id’] , and using $id instead would be a good idea (just using $_GET[‘id’] like you are now is also prone to SQL injections, using $id isn’t, because you applied mysql_real_escape_string to that).


while ($row = mysql_fetch_assoc($res))

You have LIMIT 1 in your query, so you either get 1 result, or zero results. So the while will always ever just run one (since $num==0 was already caught earlier). Remove the while, and just leave $row = mysql_fetch_assoc($res);


//date format
$old_date = $row['post_date'];            
$old_date_timestamp = strtotime($old_date);
$new_date = date('d.m.Y   H:i', $old_date_timestamp); 

Ehm, why not just store the timestamp in the database if that’s what you’re going to use anyway?


ob_start();
ob_end_clean();

Doesn’t do anything, drop those lines.


$output = preg_match_all('/<img.+src=[\\'"]([^\\'"]+)[\\'"].*>/i', $my1content, $matches); 
$first_img = $matches [1] [0];
if(empty($first_img)){ //Defines a default image
    $first_img = "/img/default.png";
}

This is pretty heavy, I would run it when you save the post, and save the first_img in a separate column in the database, and use that every time you request the post, instead of hauling through regex every time you request it.


<div class="single-header">
	<div class="single-title">'.$title.'</div>	
	<div class="single-tr"> '.$new_date.'</div> 
</div><!-- single header -->
<div class="single-print"></div><!-- print -->
	<div class="single-content">
	<div class="single-img">
	<img src="timthumb.php?src='.$first_img.'&amp;h=223&amp;w=395&amp;zc=1" alt="" />
	</div>
	<div class="single-text">'.$mycontent.' </div>
</div> <!-- content -->

That’s waaaay too many divs and waaay too verbose classnames… Try this:


<div class="single">
	<div class="header">
		<h2>'.$title.'</h2>	
		'.$new_date.' 
	</div><!-- single header -->
	<div class="single-print"></div><!-- print -->
	<div class="content">
		<img src="timthumb.php?src='.$first_img.'&amp;h=223&amp;w=395&amp;zc=1" alt="" />
		<div>'.$mycontent.' </div>
	</div> <!-- content -->
</div> <!-- single -->

Of course you’d need the CSS to go it with, but that should pretty much speak for itself. This is way more generic and a lot easier to modify if you ever need to.


echo 'Dont exist';

You may want to consider using the [fphp]header function[/fphp] to throw an HTTP 404 Not Found here, to indicate to search engines they shouldn’t bother coming back to the requested URL; the article is gone.

hello,

thank you for the great answer, it will be very usefull.

too many divs and classes are bad for the web? what can they cause ?
also i have a big problem too.

by adding ’ at my adress at browser example : http://mywebsite.com/singel?id=24

i get an error :

You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near ‘\’ LIMIT 1’ at line 1

This is showed everywhere if i put ’ after and id showed at my web.

Please help, what is wrong, and how it can be fixed. Thank you

Thank you again.

EDIT: i have changed it to : $sql = "SELECT id,post_title,post_date,post_content FROM wp_posts WHERE id=‘.$id.’";

and dont get an error any more.

Thank you, any other suggestion, will be just great.

Well they don’t really “cause” anything per se, but it’s just bad practice, because HTML is supposed to be semantic (i.e. mean something) – yours didn’t, it was just divs everywhere. Also mine is less code and uses descendant selectors, so you can easily move it around through the HTML. That’s it in a nutshell, books are writen about this subject, so I could go on and on for hours, but if you want more info you’d better get a book :slight_smile:

using " " instead of ’ ’ at the example i have given, is important ? if yes, what should i use ? thank you

I personally prefer ’ over " because I like the syntax better and it’s a bit faster*, but it doesn’t matter really. Pick the one you like.

So no, not important at all.

  • When I say “a bit”, I do mean “a bit”. The difference is so small, you won’t notice it at all; it’s probably like one 1000th of a blink of an eye, or something like that. Besides, there are better things to get speed gains than ’ vs " , for example through caching.

What about caching, can i have any suggestion what to implement at my website ? xcache and what else ? can i have some instructions about the cache please ? thank you.

Yes, xcache is an option, but there are others like APC (which will be part of the PHP core as of 5.4).

Then there’s also other mechanisms like memcache, but that’s all but too complicated to explain in a forum post. Please google for it and come back if you have any specific questions :slight_smile:

so APC is better then xcache ? this system are accelerators right ? so they make php to compile faster and show it to the visitor.

if i have installed xcache or APC, at my server, they will automatically start caching the php scripts ? or need some configurations ?

i have memcached configured and installed at my vps. i think it is working, it need any further configuration? i

what about the cache between the visitor and the server side, what you suggest to use ? or it is not needed ?

Please some info is possible.
Thank you a lot.
Here all members are very gentile. :slight_smile:

Yes, APC and xcache are both accelerators. I can’t exactly comment which one is better; both have their pros and cons, but the core PHP team seems to have chosen for APC, so that’s saying something :slight_smile:

And indeed, once you’ve installed it, it will start caching files without needing to set up anything (you can give it more RAM if you wish, but this is normally not needed).

As for memcache, you do need to configure that (how much RAM it may take) and you need to program it in to your website. Just installing it doesn’t achieve anything at all. You need to connect to it from php (using the memcache module, which you have to install) and then you can store and retrieve cache values from it. As I said before, this is too complex to explain fully. As an example take a look at the article here: Using Memcache with MySQL and PHP « Adventures in PHP / DHTML / CSS and MySQL

As for server <-> client caching, this should normally be set up on your server already. Just use a tool like firebug to see what’s happening when the page loads; probably most requests will be served from cache (with a 304 Not Modified HTTP response).

Ok a last question, and i will not bother you anymore :slight_smile:

i have a database with 85.000 news, and i am creating a new webiste,
to avoid some server load, i will devide the news, the latest 1000 news will be at the live webiste and the other 84.000 news will be on a sub domain, called archive.mydomain.com
wanted to ask if this will affect my alexa and google ranking, if yes, what can i do to improve or let them as they are.

Best Regards
AvinD

I have no idea about that last one; you’d better ask that question in our Search Engine Optimization board :slight_smile:

Ok thank you