[SOLVED] convert special phrases to images

[AviNahum]

hey,

img using this function:

public function convert($txt) {
global $DB, $cms;

//--------------------------------
// Convert images
//--------------------------------

	while( preg_match("/<{img_(.+?)\}>/" , $txt ) )
	{
		// Getting image id
		$img_id = preg_replace("@^(.+?)\<{img_(.+?)\}>(.+?)\$@is", "\\2", $txt );

		// Select the image info from DB
		$DB->query("SELECT * FROM cms_gal_images WHERE id=$img_id");
		$img_info = $DB->fetch_row();

		$img_url = $img_info['url'];

		// Replacing
		$txt = preg_replace( "/<{img_(.+?)\}>/", "<img src='{$cms->admin_dir}/{$cms->gall_dir}/{$img_url}'>", $txt );
	}

return $txt;
}

 

what i meant to do is simple... the function get a string, and if there are phrase like <{img_12}> so it convert in to <img src="12.gif">...

12 is the image id, its might be replaced with any number...

i getting the image id (look at the $img_id var)

and then i select the image data (image title, image description, etc) from the database...

its works fine only if there only 1 phrase of <{img_13}>

 

but lets assume that the string is:

<p>

some text

<{img_6}>

some more text

<{img_4}>

</p>

 

then i got a problem, the first phrase (<{img_6}>) replaced with the image id 6...

but the second (<{img_4}>) should be replaced with image id 4, but instead its replaced with the first phrase data (image id 6)...

 

Thanks!

 

***sorry for poor english***

[RussellReal]

preg_replace("/<\{img_(\d+?)\}>/", "<img src='\\1.gif' />", $theSource);

[AviNahum]

thanks for trying to help,

but did you look at my code?

[RussellReal]

what I just gave you should work better than what you posted..

[AviNahum]

and you have an idea how to fix it? i tried to use while loop but it's wont work

[thebadbad]

You are starting the regex engine way more times than necessary. And you shouldn't use preg_replace() to extract a value. Consider this:

 

<?php
public function convert($txt) {
global $DB, $cms;
//grab all image tag IDs
preg_match_all('~<{img_([0-9]*?)}>~i', $txt, $matches);
//loop through IDs, query the database, and replace the tags
foreach ($matches[1] as $id) {
	$DB->query(sprintf("SELECT * FROM `cms_gal_images` WHERE `id` = '%s'", mysql_real_escape_string($id));
	$img_info = $DB->fetch_row();
	$img_url = $img_info['url'];
	//replace
	$txt = str_replace('<{img_' . $id . '}>', "<img src=\"{$cms->admin_dir}/{$cms->gall_dir}/{$img_url}\" />", $txt);
}
return $txt;
}
?>

 

Should be much more efficient.

[AviNahum]

Thanks alot!

its works great!

[thebadbad]

You're welcome. I hope you can follow my code and learn something (you were using excessive regular expressions where only one was necessary ). Else just ask.

[RussellReal]

why are you using preg_match to match the occurances then doing ANOTHER loop AFTER the regular expressions engine.. and THEN using str_replace.. you should just do it all in 1 line like I showed him.. do a benchmark bro

[thebadbad]

why are you using preg_match to match the occurances then doing ANOTHER loop AFTER the regular expressions engine.. and THEN using str_replace.. you should just do it all in 1 line like I showed him.. do a benchmark bro

 

If you had taken the time to read our posts, you would know that the OP needs different strings depending on the ID, for the replacement. The script could probably be optimized by getting all the image URLs from the database first (if there aren't too many), storing them in an array with the IDs as keys and then do a single preg_replace(), accessing the URLs in the replacement via the ID key in the before-mentioned array. Feel free to post a more efficient solution that does what the OP wants.

[AviNahum]

i got one qusetion,

foreach ($matches[1] as $id) {

 

why its $matches[1] and not $matches?

sorry, i'm new for PHP

[RussellReal]

you could also use WHERE id IN (..,..,..)

 

to select all the images all at once instead of reading from the db 100 times

[thebadbad]

$matches is a two-dimensional array. preg_match_all() stores all full pattern matches as elements in the array $matches[0], all matches from the first captured parenthesized subpattern as elements in the array $matches[1], etc. In our pattern ~<{img_([0-9]*?)}>~i, the IDs are captured within the first set of parentheses, and therefore stored in the array $matches[1].

[AviNahum]

ohh i see...

thanks again!

[thebadbad]

And Russell is right that we can optimize it even more by only querying the database once. I just hesitated to implement it before, because you were using a class to query the database. Here's the 'fully' optimized code, using the regular mysql_query() etc.:

 

<?php
public function convert($txt) {
//make sure the mysql connection is accessible inside the function
global $DB, $cms;
//grab all image tag IDs
preg_match_all('~<{img_([0-9]*?)}>~i', $txt, $matches);
$result = mysql_query('SELECT `id`, `url` FROM `cms_gal_images` WHERE `id` IN (' . implode(', ', $matches[1]) . ')');
$data = array();
while ($row = mysql_fetch_assoc($result)) {
	$data['<{img_' . $row['id'] . '}>'] = "<img src=\"{$cms->admin_dir}/{$cms->gall_dir}/{$row['url']}\" />";
}
return str_replace(array_keys($data), $data, $txt);
}
?>

 

Uses the regex engine once, queries the database once and runs the fast str_replace() once (on arrays though ). I know the code is not fitting with your current code (using a class, most importantly), but it shouldn't be that difficult to adapt.