walermo Posted September 12, 2013 Share Posted September 12, 2013 (edited) I have the following code to create a report of contractor times: I basically displays jobs serviced by the contractors during a specific time range. <?php // fetch all job_dates within the specified date range ($startdate - $enddate) // select distinct job_date from jobs where job_start between ? and ? order by job_date $rep_dates = $users->get_job_dates($startdate, $enddate); foreach ($rep_dates as $rep_date_group) { // fetch all contractor jobs for each job_date in the date range // select * from jobs where contractor_id = ? and job_date = ? order by job_start $rep_group = $users->get_jobs($contractor_id, $rep_date_group['job_date']); if ($rep_group) { foreach ($rep_group as $rep_group_row) { # process results here } } } The code works and the results are properly processed. All database queries are made using prepared statements with PDO. Is there are more efficient way to do this? Thanks for your help! Edited September 12, 2013 by walermo Quote Link to comment https://forums.phpfreaks.com/topic/282102-coding-suggestion/ Share on other sites More sharing options...
requinix Posted September 12, 2013 Share Posted September 12, 2013 Very likely. What's the code for get_job_dates() and get_jobs()? Quote Link to comment https://forums.phpfreaks.com/topic/282102-coding-suggestion/#findComment-1449259 Share on other sites More sharing options...
walermo Posted September 12, 2013 Author Share Posted September 12, 2013 They are the PDO queries, which I summarily referenced in the code comments above: public function get_job_dates($start, $end) { $query = $this->db->prepare("SELECT DISTINCT job_date FROM jobs WHERE job_date BETWEEN ? AND ? ORDER BY job_date ASC"); $query->bindValue(1, $start); $query->bindValue(2, $end); try { $query->execute(); return $query->fetchAll(); } catch (PDOException $e) { die($e->getMessage()); } } public function get_jobs($contractor, $date) { $query = $this->db->prepare("SELECT *, TIMEDIFF(job_end, job_start) AS job_time, users.first_name AS int_name, users.last_name AS int_last, users.rate AS int_rate, FROM jobs LEFT JOIN users ON jobs.contractor_id = users.id WHERE job_date = ? AND contractor_id = ? AND entry = 1 ORDER BY job_date ASC, job_start ASC"); $query->bindValue(1, $date); $query->bindValue(2, $interp); try { $query->execute(); } catch (PDOException $e) { die($e->getMessage()); } return $query->fetchAll(); } Quote Link to comment https://forums.phpfreaks.com/topic/282102-coding-suggestion/#findComment-1449309 Share on other sites More sharing options...
requinix Posted September 13, 2013 Share Posted September 13, 2013 (edited) Eliminate the requirement of passing $date to get_jobs() so you'll get all jobs for all dates (or create a new function which doesn't use a date). Which is what your first bit of code is trying to do. Looks like you're grouping by a date, maybe with some kind of table or heading for each date. What is the rest of the code? It can almost certainly be modified so that it intelligently detects when the date changes between rows and can manage the tables/headings accordingly. Edited September 13, 2013 by requinix Quote Link to comment https://forums.phpfreaks.com/topic/282102-coding-suggestion/#findComment-1449317 Share on other sites More sharing options...
kicken Posted September 13, 2013 Share Posted September 13, 2013 You only need one query: SELECT * , TIMEDIFF(job_end, job_start) AS job_time , users.first_name AS int_name , users.last_name AS int_last , users.rate AS int_rate FROM jobs LEFT JOIN users ON jobs.contractor_id = users.id WHERE job_date BETWEEN ? AND ? AND contractor_id = ? AND entry = 1 ORDER BY job_date ASC , job_start ASC Queries in loops like you have right now are generally a bad idea and begin to kill your app's performance, even at only a few loop iterations. You also should not be using * in your select list. It also will hurt performance by selecting unnecessary data, and also makes the queries less friendly by requiring you (or someone else) to reference the database table structure to find out what data is being returned from said query. Lastly, your query in get_jobs has a syntax error in it. There is an extra , before FROM. Quote Link to comment https://forums.phpfreaks.com/topic/282102-coding-suggestion/#findComment-1449319 Share on other sites More sharing options...
walermo Posted September 13, 2013 Author Share Posted September 13, 2013 Hi kicken! Indeed, that syntax error was left over when I copied over the function code, which I trimmed for convenience. Also duly noted your suggestion about not using * in my select list. I actually don't use it and again, i put it here because I trimmed the code. In retrospect, that was not a good idea when coming for coding help. Requinix: below is the rest of my code. Thanks again for your help and suggestions! // functions being called for database access public function get_job_report_dates($start, $end) { $query = $this->db->prepare("SELECT DISTINCT job_date FROM jobs WHERE job_date BETWEEN ? AND ? AND entry = 1 ORDER BY job_date ASC"); $query->bindValue(1, $start); $query->bindValue(2, $end); try { $query->execute(); return $query->fetchAll(); } catch (PDOException $e) { die($e->getMessage()); } } public function get_job_report_interp($contractor, $date) { $query = $this->db->prepare("SELECT job_num, job_date, contractor_id, job_facility, job_procedure, job_start, job_end, TIMEDIFF(job_end, job_start) AS job_time, users.first_name AS contractor_name, users.last_name AS contractor_last, users.rate AS contractor_rate, FROM jobs LEFT JOIN users ON jobs.contractor_id = users.id) WHERE job_date = ? AND contractor_id = ? AND entry = 1 ORDER BY job_date ASC, job_start ASC"); $query->bindValue(1, $date); $query->bindValue(2, $contractor); try { $query->execute(); } catch (PDOException $e) { die($e->getMessage()); } return $query->fetchAll(); } ##################################### <?php /* billing_report_by_contractor */ include 'assets/inc/functions.inc'; include 'assets/inc/header.inc'; ?> <body> <?php include 'assets/inc/menu.admin.inc'; if (isset($_POST['next']) || isset($_POST['generate'])) extract($_POST); ?> <section class='container'> <div class='well hero-unit'> <div class='well'> <h3>Job Reports by contractor</h3> <h4><?php echo (isset($_POST['generate']) ? "Showing jobs for the following period" : "Please select a date range"); ?>:</h4> <form action='<?php echo $_SERVER['PHP_SELF']; ?>' method='post' name='formid' class='form-inline'> <input type='text' name='startdate' id='startdate' <?php echo ((isset($_POST['next']) || isset($_POST['generate'])) ? "placeholder='" . datef($startdate, "m/d/Y") . "' value='" . datef($startdate, "m/d/Y") . "' readonly" : "placeholder='Start Date' required"); ?>> <input type='text' name='enddate' id='enddate' <?php echo ((isset($_POST['next']) || isset($_POST['generate'])) ? "placeholder='" . datef($enddate, "m/d/Y") . "' value='" . datef($enddate, "m/d/Y") . "' readonly" : "placeholder='End Date' required"); ?>> <?php if (!isset($_POST['next']) && !isset($_POST['generate'])) { ?> <input type='hidden' name='by' value='contractor'> <button type='submit' class='btn btn-vlb' name='next'>Next</button> <?php } //!isset($_POST['next']) && !isset($_POST['generate']) ?> <?php if (isset($_POST['startdate'])) { extract($_POST); $startdate = datef($startdate, "Y-m-d"); $enddate = datef($enddate, "Y-m-d"); $report_select = $users->get_report_contractor($startdate, $enddate); ?> <h4><?php echo (isset($_POST['generate']) ? "Select another contractor" : "...and now select a contractor"); ?></h4> <select name='contractorid'> <?php foreach ($report_select as $report_option) { echo "<option value='" . $report_option['contractor_id'] . "'>" . $report_option['contractor_name'] . " " . $report_option['contractor_last'] . "</option>"; } //$report_select as $report_option ?> </select> <input type='hidden' name='startdate' value='<?php echo $startdate; ?>'> <input type='hidden' name='enddate' value='<?php echo $enddate; ?>'> <button type='submit' class='btn btn-vlb' name='generate'>Generate Report!</button> </form> <?php } //isset($_POST['startdate']) if (isset($_POST['generate'])) { $records = 1; // for numbering rows $contractor_total_time = 0; $contractor_total_up_time = 0; $contractor_total_fee = 0; $contractor_total_up_fee = 0; $rowdata = ''; // get distinct job dates with contractor job time entries in the selected time range $report_dates = $users->get_job_dates($startdate, $enddate); foreach ($report_dates as $report_date_group) { $date_total_time = 0; $date_total_up_time = 0; $contractor_date_fee = 0; $contractor_date_up_fee = 0; $job_total = 0; // get jobs serviced by the selected contractor for each date found $report_group = $users->get_jobs($contractorid, $report_date_group['job_date']); if ($report_group) { foreach ($report_group as $report_group_row) { $contractor_name = $report_group_row['contractor_name'] . " " . $report_group_row['contractor_last'] . " (" . $report_group_row['job_lang'] . ")"; $job_up_time = ''; $job_date = datef($report_group_row['job_date'], "m/d/Y"); $job_start = datef($report_group_row['job_start'], "g:i a"); $job_end = datef($report_group_row['job_end'], "g:i a"); $job_time = decimals($report_group_row['job_time']); $job_up_time = ($job_time < 2) ? 2 : $job_time; // two-hour minimum $job_rate = $report_group_row['contractor_rate']; $job_total = $job_time * $job_rate; $job_up_total = $job_up_time * $job_rate; // add up totals $contractor_total_fee += $job_total; $contractor_date_fee += $job_total; $contractor_total_up_fee += $job_up_total; $contractor_date_up_fee += $job_up_total; $contractor_total_time += $job_time; $date_total_time += $job_time; $date_total_up_time += $job_up_time; $contractor_total_up_time += $job_up_time; $rowdata .= "<tr> <td>" . $records . "</td> <td>" . $report_group_row['job_num'] . "</td> <td>" . substr($report_group_row['job_facility'], 0, 6) . "</td> <td>" . $report_group_row['job_dept'] . "</td> <td>" . substr($report_group_row['job_procedure'], 0, 10) . "</td> <td>" . $job_date . "</td> <td style='text-align:right;'>" . $job_start . "</td> <td style='text-align:right;'>" . $job_end . "</td> <td style='text-align:right;'>" . number_format($job_time, 2, '.', '') . "</td> <td style='text-align:right;'>" . cash($job_total) . "</td> <td style='text-align:right;'>" . ($job_time < 2 ? "<span style='color:blue'><em>" . number_format($job_up_time, 2, '.', '') . "</em></span>" : number_format($job_up_time, 2, '.', '')) . "</td> <td style='text-align:right;'>" . ($job_time < 2 ? "<span style='color:blue'><em>" . cash($job_up_total) . "</em></span>" : cash($job_up_total)) . "</td> </tr>"; $records++; } //$report_group as $report_group_row $rowdata .= "<tr> <td colspan=9 style='text-align:right;color:green;'><em>Date total</em></td> <td style='text-align:right;color:green;'><em>" . number_format($date_total_time, 2, '.', '') . "</em></td> <td style='text-align:right;color:green;'><em>" . cash($contractor_date_fee) . "</em></td> <td style='text-align:right;color:green;'><em>" . number_format($date_total_up_time, 2, '.', '') . "</em></td> <td style='text-align:right;color:green;'><em>" . cash($contractor_date_up_fee) . "</em></td> </tr>"; } //$report_group } //$report_dates as $report_date_group ?> <h4>Job report for <?php echo $contractor_name; ?></h4> <table class="table-bordered table-condensed" style="font-size: 12px; line-height:12px;"> <thead class="cf"> <tr> <th>#</th> <th>Job#</th> <th>Facility</th> <th>Dept</th> <th>Procedure</th> <th>Job Date</th> <th>Start</th> <th>End</th> <th>Time</th> <th>Total</th> <th>Up-time</th> <th>Total Up</th> </tr> </thead> <tbody> <?php echo $rowdata; ?> </tbody> <tfoot> <tr> <td colspan=10 style='text-align:right;'><strong>Total for <?php echo $contractor_name; ?></strong></td> <td style='text-align:right;'><strong><?php echo number_format($contractor_total_time, 2, '.', ''); ?></strong></td> <td style='text-align:right;'><strong><?php echo cash($contractor_total_fee); ?></strong></td> <td style='text-align:right;'><strong><?php echo number_format($contractor_total_up_time, 2, '.', ''); ?></strong></td> <td style='text-align:right;'><strong><?php echo cash($contractor_total_up_fee); ?></strong></td> </tr> </tfoot> </table> <?php } //isset($_POST['generate']) ?> </div> <?php include('assets/inc/footer.inc'); ?> </div> </section> </body> </html> Quote Link to comment https://forums.phpfreaks.com/topic/282102-coding-suggestion/#findComment-1449394 Share on other sites More sharing options...
walermo Posted September 18, 2013 Author Share Posted September 18, 2013 Gah! I just noticed that I left that typo in the code again Sorry kicken Quote Link to comment https://forums.phpfreaks.com/topic/282102-coding-suggestion/#findComment-1450149 Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.