Is there a better way to write this function?

[c_shelswell]

I was just wondering if there was a better way to write this. My query should return a few results and i'm adding it to an array in the foreach but i'm having to use an $i++; to add the next item to the array. The code works and returns what i'm after  just wondering if there's a slightly better way.

thanks

[code]function get_media_titles($mediaId)
{
$conn = db_connect();
$i=0;
foreach ($mediaId as $key => $id)
{
$query="select media.title from media left join purchases using(media_id) where purchases.media_id='$id' limit 1";
$result= mysql_query($query);
if (!$result)
return false;
$num_rows = @mysql_num_rows($result);

if ($num_rows == 0)
return false;

if($result)
{
$titles[$i] = mysql_result($result, 0, 'title');
}
$i++;
}
return $titles;
}[/code]

[theironchef]

looks good. if it ain't broke don't fix it  :P

[kenrbnsn]

You can improve this function in the following ways:
[list]
[*] Always initialize your arrays.
[*] You don't need to use the index, "$i". Just use the empty index, " [] ", which automagically adds an entry to the end of an array.
[*] Use the function mysql_fetch_assoc() instead of mysql_result()
[*] You don't need the "if ($result)" check since you've already returned when the $result as false
[/list]

[code]<?php
function get_media_titles($mediaId)
{
$conn = db_connect();
$titles = array();
foreach ($mediaId as $key => $id)
{
$query="select media.title from media left join purchases using(media_id) where purchases.media_id='$id' limit 1";
$result= mysql_query($query);
if (!$result)
return false;
$num_rows = @mysql_num_rows($result);

if ($num_rows == 0)
return false;
                $rw = mysql_fetch_assoc($result);
$titles[] = $rw['title'];
}
return $titles;
}
?>[/code]

Ken

[c_shelswell]

excellent thanks very much i thought there must be a way of getting rid of the $i counters it didn't look very elegant.

thanks again.

[theironchef]

lol i didnt even look at the code sry.... but the other guy how come you use mysql_fetch_assoc instead of his version of result? they both end up wokring the same way right? (im a n00b  :-[)

[kenrbnsn]

In this case it doesn't matter much, just personal preference, but when you're using many fields from the row, using one of the functions that fetches the entire row in one swell foop ( :) ), is quicker and more efficient that getting each field one at a time.

Ken