How can I make this run faster?

Coding Critique is the place to post source code for peer review by other members of DevNetwork. Any kind of code can be posted. Code posted does not have to be limited to PHP. All members are invited to contribute constructive criticism with the goal of improving the code. Posted code should include some background information about it and what areas you specifically would like help with.

Popular code excerpts may be moved to "Code Snippets" by the moderators.

Moderator: General Moderators

Post Reply
condoravenue1
Forum Commoner
Posts: 30
Joined: Fri Dec 03, 2010 10:24 pm

How can I make this run faster?

Post by condoravenue1 »

I have some code that takes user input, like 'John 3:16', and puts 'John' to a variable, '3' to a variable and '16' to a variable. It then inserts values from one mysql table to another according to these three variables. If the input isn't spelled correctly, or if the verse doesn't exist in the Bible, or if that verse was already added, then I inform them.

Is there anyway to make this faster?

Code: Select all

$username = mysql_real_escape_string($_SESSION['username']);
if(isset($_POST['reference'])){
	
	$reference = $_POST['reference'];
	$non_letters = array(':', '1', '2', '3', '4', '5', '6', '7', '8', '9', '0', '-');
	if (substr($reference, 0, 1) == '1' || substr($reference, 0, 1) == '2' || substr($reference, 0, 1) == '3') {$space_book = substr($reference, 0, 1);} else {$space_book = '';}
	$book_name = rtrim($space_book . str_replace($non_letters, '', $reference));
	$chapter_number = substr($reference, strlen($book_name) + 1, strpos($reference, ':') - strlen($book_name) - 1);
	if(strpos($reference, '-') !== false) {
		$first_verse = substr($reference, strpos($reference, ':') + 1, strpos($reference, '-') - strpos($reference, ':') - 1);
		$last_verse = substr($reference, strpos($reference, '-') + 1, strlen($reference) - strpos($reference, '-') - 1);
		for ($verse_number = $first_verse; $verse_number <= $last_verse; $verse_number++){
			$get_verse = mysql_query("SELECT * FROM esv WHERE book = '$book_name' AND chapter = '$chapter_number' AND verse = '$verse_number'");
			while($row = mysql_fetch_array($get_verse)){$esv_book = $row['book']; $esv_type = $row['type']; $esv_testament = $row['testament']; $esv_book_index = $row['book_index']; $esv_chapter = $row['chapter']; $esv_verse = $row['verse']; $esv_content = $row['content'];}
			$duplicate = mysql_query("SELECT * FROM verses_$username WHERE book = '$book_name' AND chapter = '$chapter_number' AND verse = '$verse_number'");
			if(mysql_num_rows($get_verse) != 0){
				if(mysql_num_rows($duplicate) != 0){
					$added = 1;
					}
				else{
					mysql_query("INSERT INTO verses_$username (testament, type, book, book_index, chapter, verse, content, date) VALUES ('$esv_testament', '$esv_type', '$esv_book', '$esv_book_index', '$esv_chapter', '$esv_verse', '$esv_content', now())");
					$stat = mysql_query("SELECT * FROM stats");
					while ($row = mysql_fetch_array($stat)) {$total_verses = $row['total_verses'] + 1;}
					mysql_query("UPDATE stats SET total_verses = $total_verses");
					}
				}
			else{
				$not_a_verse = 1;
				}
			}
		}
	else{
		$verse_number = substr($reference, strpos($reference, ':') + 1, strlen($reference) - strpos($reference, ':') - 1);
		
		$get_verse = mysql_query("SELECT * FROM esv WHERE book = '$book_name' AND chapter = '$chapter_number' AND verse = '$verse_number'");
		while($row = mysql_fetch_array($get_verse)){$esv_book = $row['book']; $esv_type = $row['type']; $esv_testament = $row['testament']; $esv_book_index = $row['book_index']; $esv_chapter = $row['chapter']; $esv_verse = $row['verse']; $esv_content = $row['content'];}
		$duplicate = mysql_query("SELECT * FROM verses_$username WHERE book = '$book_name' AND chapter = '$chapter_number' AND verse = '$verse_number'");
		if(mysql_num_rows($get_verse) != 0){
			if(mysql_num_rows($duplicate) != 0){
				$added = 1;
				}
			else{
				mysql_query("INSERT INTO verses_$username (testament, type, book, book_index, chapter, verse, content, date) VALUES ('$esv_testament', '$esv_type', '$esv_book', '$esv_book_index', '$esv_chapter', '$esv_verse', '$esv_content', now())");
				$stat = mysql_query("SELECT * FROM stats");
				while ($row = mysql_fetch_array($stat)) {$total_verses = $row['total_verses'] + 1;}
				mysql_query("UPDATE stats SET total_verses = $total_verses");
				}
			}
		else{
			$not_a_verse = 1;
			}
		}
	}
if(isset($_GET['delete_verse'])) {
	$verse_id = $_GET['delete_verse'];
	mysql_query("DELETE FROM verses_$username WHERE id = $verse_id");
	$stat = mysql_query("SELECT * FROM stats");
	while ($row = mysql_fetch_array($stat)) {$total_verses = $row['total_verses'] - 1;}
	mysql_query("UPDATE stats SET total_verses = $total_verses");
	}
User avatar
VladSun
DevNet Master
Posts: 4313
Joined: Wed Jun 27, 2007 9:44 am
Location: Sofia, Bulgaria

Re: How can I make this run faster?

Post by VladSun »

Sounds like a regexp problem to me.E.g. for 'John 3:16' format you have:

Code: Select all

if (preg_match('#^([a-zA-Z -]+)\s(\d+):(\d+)$#', $reference, $matches))
{
   print_r($matches);
}
Also, I haven't read the code in depth, but it seems you can put most of the data search into the SQL code, not in the PHP code.
There are 10 types of people in this world, those who understand binary and those who don't
User avatar
Mordred
DevNet Resident
Posts: 1579
Joined: Sun Sep 03, 2006 5:19 am
Location: Sofia, Bulgaria

Re: How can I make this run faster?

Post by Mordred »

"SELECT * FROM verses_$username ...
You create a new table for each user?

No. Have one table (say verses) with all the verses for all the users (whatever that means in your application). This means, have a column user_id that references whose 'verse' that is. Create an index on that column, so that queries with " WHERE `user_id`=5" would run faster.

Also, mysql_real_escape_string() will not protect you against SQL injection in cases like this, where you are escaping something else than a value. Check the article in my signature for details if you want, but the situation is best remedied by severely refactoring the code not to use multiple tables like that.

Also, you have things like that:

Code: Select all

        $verse_id = $_GET['delete_verse'];
        mysql_query("DELETE FROM verses_$username WHERE id = $verse_id");
(I'm sure I've already directed you once at reading about SQL injection)
Post Reply