duplicate db results

[phpWilliam]

Hi there, I know I'm doing something stupid, but i've tried for hours to try and figure this out and I just cant seem to figure out why tutorials i've seen seem to work yet my own code doesnt.

basically i've got two functions: getAll() and get($id) inside a class, calling getAll, returns an array with one of the same record as opposed to all the records (which is my intended result).  Can anyone help? thanks.

 

class Test{
        protected $_db;
        protected $_table = "table";
        
        protected $_friends = array();
        
        public function __construct($db) {
            $this->_db = $db;
        }
        
        public function getAll()
        {
            $db = $this->_db;
            $sql = "SELECT * FROM $this->_table";
            $db->query($sql);
            while($data = $db->fetch_array())
            {
                $this->_friends[$data['id']] = $this->get($data['id']);
            }
            return $this->_friends;
        }
        
        public function get($id)
        {
            $db = $this->_db;
            $sql = "SELECT * FROM $this->_table WHERE id='$id'";
            $db->query($sql);
            $data = $db->fetch_array();
            
            return (object)$data;
            
        }
}

$test = new Test(new mysqlConnClass());
$arrayOfAllAsObjects = $test->getAll();

 

[silkfire]

Try running your query in PHPMyAdmin

[Psycho]

You are using a separate class for your queries, so I can't be sure. But, . . .  are you sure about what is returned from $db->fetch_array()? If it is similar to mysql_fetch_array(), then it is returning an array with all fields associated by their field name AND with numerically based indexes. However, if that method does not return the values indexed by field name, then there is no value for "$data['id']". So, when you create the return array using

            while($data = $db->fetch_array())
            {
                $this->_friends[$data['id']] = $this->get($data['id']);
            }

 

The result of $data['id'] is 0, so each record is overwriting the last one. So you only get the last record when the loop is complete. Now, if $db->fetch_array() does returned results with named indexes then I think you need to double check the name of the id field. Are you positive it is 'id'? Make sure it isn't a different name or doesn't have a different case (i.e. "ID"). If the field name is not exactly the same then you would have the same problem as I described above.

[phpWilliam]

Hi Guys,

Thanks for the replies.

Sorry the database class I'm using is just a basic wrapper, the $db->fetch_array(), works in the same way as the mysql_fetch_array() function.

 

It appears the same $db is being used when it passed into the get() method - I thought these were private variables to that method...

 

If I modify my code in getAll() to:

 

            $db = $this->_db;
            $sql = "SELECT * FROM $this->_table WHERE `accepted`='1' ORDER BY `position` ASC";
            $db->query($sql);
            $this->_friends = array();
            
            $ids = array();
            while($data = $db->fetch_array())
            {
                $ids[] = $data['id'];
            }
            foreach($ids as $id)
                $this->_friends[$id] = $this->get($id);
            return $this->_friends;

 

I get the intended result.

Could this be an issue deeper in the db wrapper I am fetching from $this->_db?  I assumed the $db being used in get($id) would be seperate from the $db being used in getAll() however they appear to be causing problems with each other..

 

Does anyone have any pointers? As the code above is a little messier than I would like - Thanks,

[gizmola]

Yes it looks like an issue with your db class, which is not something you provided.  You pass this in, but it is not clear what it is exactly, but in your original routine, you did a query, then went into a fetch loop, and tried inside that fetch loop to do a second query using the same db object.  One would clobber the other.  It really doesn't make any sense in your code, as to why in your getAlll you would query against the same table for every row, when your outer query has already queried every row.  I don't understand why you aren't just doing this in your getAll().

 

$this->_friends = array();
while($data = $db->fetch_array())
{
     $this->_friends[$data['id']] = $data;
  }
  return $this->_friends;

 

[phpWilliam]

I thought it was "best practise" to separate the getAll and getSingle methods? As I can use the same getSingle method when loading (in my case) a friend on my site.

[gizmola]

It's good to have a method that gets a single row -- it's just not good to use that method to get every row in a result set, when you already have that result set.  That is simply redundant not to mention, terribly inefficient. 

[phpWilliam]

Thanks gizmola, I'll take the last comment on the chin about my "code efficiency" 

 

Think I can mark this as solved, although a more efficient / refactored class would be nice if someone could be so kind.

 

Thanks for the warm welcome to phpfreaks, the replies have been more than I expected

 

[Psycho]

you did a query, then went into a fetch loop, and tried inside that fetch loop to do a second query using the same db object.

I totally missed that. I thought he was simply appending the result to an array - didn't see he was calling another method. Doh!

 

 

Think I can mark this as solved, although a more efficient / refactored class would be nice if someone could be so kind.

 

gizmola provided a solution for you:

        public function getAll()
        {
            $db = $this->_db;
            $sql = "SELECT * FROM $this->_table";
            $db->query($sql);
            while($data = $db->fetch_array())
            {
                $this->_friends[$data['id']] = $data; //<--Changed line
            }
            return $this->_friends;
        }