2
\$\begingroup\$

I'm trying to figure out whether I should do the following:

This class (over-simplified version) retrieves information about an order by having 1 main query function that select a specific column from the table and throws out the value for that column and that specific order ID.

class jobinfo
{

    protected $id;

    function __construct($job_id)
    {
        $this->id-$job_id;

    }

    protected function query_s($table, $col = '*', $where_col , $where, $return_col)
    {
        $SQL = mysql_query("SELECT $col FROM ".$table." WHERE $where_col='$where' LIMIT 1") or die(mysql_error());
        $data = mysql_fetch_array($SQL);
        return $data[$return_col];  
    }

    public function qtyOrdered()
    {
        return $this->query_s('job_sheet','qty', 'job_id',$this->id, 1, 'qty');
    }   

    public function dueDate()
    {
        return $this->query_s('job_sheet','due', 'job_id',$this->id, 1, 'due');
    }   

}

The way I have it set up now is that I have this running in a while loop over 10,000 different orders like so:

$sql = mysql_query("SELECT job_id FROM job_sheet");
while($data = mysql_fetch_array($sql))
{
    $job_data = new jobinfo($data['job_id']);

    //table goes here to output values
    echo $job_data->dueDate();
    echo $job_data->qtyOrdered();
    ///
}

The one issue with this is that for every row that comes up, there are 2 queries that are happening in the back: one for the quantity and one for the due date. This ultimately causes to slow things down, especially when I'm outputting almost 11 columns for 11 different things. It adds up to a lot of queries per row.

My question is, should I go with something like this:

class jobinfo
{

    protected $id;
    protected $due;
    protected $qty;

    function __construct($job_id)
    {
        $this->id-$job_id;

        $this->retrieve();

    }

    protected function retrieve()
    {
        $sql = mysql_query("SELECT qty, due FROM job_sheet WHERE job_id=".$this->id." LIMIT 1");
        $data = mysql_fetch_array($sql);
        $this->due = $data['due'];
        $this->qty = $data['qty'];

        /// etc etc etc

    }


    public function qtyOrdered()
    {
        return $this->qty;
    }   

    public function dueDate()
    {
        return $this->due;
    }               
}

Which would do only one query per row in this case.

My dilemma is that the only time I find the second class more efficient is when some of this information is called via Ajax on a refresh rate of 5 seconds to be displayed somewhere. There are 15 users and most of the stay on one page that refreshes data every 5 seconds for statistical purposes.The other time is when the user searches for something in a custom search engine. The rest of the site is primarily showing only a few things and don't need to search the entire database to retrieve one specific value like the second class would have been doing. And so, this is the reason why I've set up my class as the first one: 'retrieve as you go' basically:

$job = new jobinfo(123); echo $job->qty();

I'm not sure what to do at this point and need some advice/suggestions.

\$\endgroup\$
3
  • \$\begingroup\$ What you should do first and foremost is using prepared statements instead of string concatenations! \$\endgroup\$
    – fge
    May 24, 2013 at 5:19
  • \$\begingroup\$ Please do not use mysql in new code, it is deprecated. Use mysqli or PDO instead. Use prepared statements. I'd also use a general function to retrieve data from the database and call it from other functions, it reduces the amount of queries throughout the code, making it better to read an maintain. \$\endgroup\$
    – Alex
    May 24, 2013 at 8:22
  • \$\begingroup\$ I completely agree with all this. It's just a matter of re-writing most of the code which would seem impossible. As for the new code, I've implemented PDO. Not sure what to do about old code... \$\endgroup\$
    – Dimitri
    May 24, 2013 at 12:48

1 Answer 1

1
\$\begingroup\$

Doing this in a loop will be slow, and doubling the calls by retrieving each column with a different query will make things worse.

Why not use a JOIN between your original query, something like this:-

SELECT database.job_id, job_sheet.qty, job_sheet.row FROM database INNER JOIN job_sheet ON database.job_id = job_sheet.job_id

In your loop this would mean a single query rather than ~20000 queries.

If you do need to do a separate query for each time through the loop then a prepared statement would help but peformance will likely still be poor.

\$\endgroup\$
3
  • \$\begingroup\$ I changed the while loop: it's not SELECT job_id FROM database it should read SELECT job_id FROM job_sheet. Doing a query for every column multiplied by every row is insanity. I think perhaps what I'll do is doing an INNER JOIN but within my class; retrieving all the information first... I wonder if that would improve much. But that would be I would have to join almost 3 tables now for each row in my while loop. \$\endgroup\$
    – Dimitri
    May 24, 2013 at 17:51
  • \$\begingroup\$ If it is a join on index int columns then it should be fast. I wouldn't worry about the performance of a JOIN as it will almost certainly be massively faster than a separate query to the 2nd table. \$\endgroup\$
    – Kickstart
    May 28, 2013 at 8:10
  • \$\begingroup\$ You're right, I think by doing the JOIN withing the class an assigning each of my variable a piece of data would be a lot better than separate queries. I was just worried about 3 JOINS for every row displayed but 3 JOINS > 11 separate queries... \$\endgroup\$
    – Dimitri
    May 28, 2013 at 19:20

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.