PHP PDO App - Rewrite PDF report based on Precinct

I am generating a PDF report for an attendance application that I am finishing up. However, I realized that the code in the report is very repetitive and takes up a LOT of lines of code and can be a major hassle to edit.

The PDF creates a list of people who are marked as present or marked as an excused absence ordered and separated by precinct per page.

Is there a better way to rewrite this code:

<?php

require("fpdf/fpdf.php");
include("../dbconnect.php");

class Report extends FPDF {
	function header() {
		$this->Image('../../img/ccrp_seal.png', 10, 6, 25, 25);
		$this->SetFont('Arial', 'B', 14);
		$this->Cell(275, 5, 'Cabarrus County GOP Convention' . ' ' . date("Y"), 0, 0, 'C');
		$this->Ln();
		$this->SetFont('Arial', '', 12);
		$this->Cell(275, 10, 'Precinct Report', 0, 0, 'C');
		$this->Ln(25);
	}
	
	function footer() {
		$this->SetY(-15);
		$this->SetFont('Arial', '', 8);
		$this->Cell(0, 10, 'Page '. $this->PageNo(), 0, 0,'R');
	}
	
	function headerPrecinct() {
		$x = $this->GetX();
		$this->SetX(21);
		$this->SetFont('Arial', 'B', 10);
		$this->Cell(20, 10, 'Precinct', 1, 0, 'C');
		$this->Cell(60, 10, 'Member Name', 1, 0, 'C');
		$this->Cell(100, 10, 'Residential Address', 1, 0, 'C');
		$this->Cell(20, 10, 'Present', 1, 0, 'C');
		$this->Cell(36, 10, 'Excused Absence', 1, 0, 'C');
		$this->Ln();
		$this->SetX($x);
		
	}
	
	function viewPrecinct_0102() {
		$this->SetFont('Arial', '', 8);
		global $pdo;
		$sql = "SELECT precinct, CONCAT(last_name, ', ', first_name, ' ', middle_name, ' ', IFNULL(suffix, ' ')) as full_name, residential_address, IF(at.member_id IS NULL, 'No', 'Yes') as 'present', IF(ab.member_id IS NULL, 'No', 'Yes') as 'absent' FROM members m LEFT JOIN attendance at ON at.member_id = m.id LEFT JOIN absence ab ON ab.member_id = m.id WHERE m.precinct = '01-02' AND (at.present = 1 OR ab.absent = 1) ORDER BY m.precinct, m.id;";
		$stmt = $pdo->prepare($sql);
		$stmt->execute();
		while($data = $stmt->fetch(PDO::FETCH_OBJ)) {
			$x = $this->GetX();
			$this->SetX(21);
			$this->Cell(20, 10, $data->precinct, 1, 0, 'C');
			$this->Cell(60, 10, $data->full_name, 1, 0, 'C');
			$this->Cell(100, 10, $data->residential_address, 1, 0, 'C');
			$this->Cell(20, 10, $data->present, 1, 0, 'C');
			$this->Cell(36, 10, $data->absent, 1, 0, 'C');
			$this->Ln();
			$this->SetX($x);			
		}
	}
	
	function viewPrecinct_0104() {
		$this->SetFont('Arial', '', 8);
		global $pdo;
		$sql = "SELECT precinct, CONCAT(last_name, ', ', first_name, ' ', middle_name, ' ', IFNULL(suffix, ' ')) as full_name, residential_address, IF(at.member_id IS NULL, 'No', 'Yes') as 'present', IF(ab.member_id IS NULL, 'No', 'Yes') as 'absent' FROM members m LEFT JOIN attendance at ON at.member_id = m.id LEFT JOIN absence ab ON ab.member_id = m.id WHERE m.precinct = '01-04' AND (at.present = 1 OR ab.absent = 1) ORDER BY m.precinct, m.id;";
		$stmt = $pdo->prepare($sql);
		$stmt->execute();
		while($data = $stmt->fetch(PDO::FETCH_OBJ)) {
			$x = $this->GetX();
			$this->SetX(21);
			$this->Cell(20, 10, $data->precinct, 1, 0, 'C');
			$this->Cell(60, 10, $data->full_name, 1, 0, 'C');
			$this->Cell(100, 10, $data->residential_address, 1, 0, 'C');
			$this->Cell(20, 10, $data->present, 1, 0, 'C');
			$this->Cell(36, 10, $data->absent, 1, 0, 'C');
			$this->Ln();
			$this->SetX($x);			
		}
	}
	
	function viewPrecinct_0107() {
		$this->SetFont('Arial', '', 8);
		global $pdo;
		$sql = "SELECT precinct, CONCAT(last_name, ', ', first_name, ' ', middle_name, ' ', IFNULL(suffix, ' ')) as full_name, residential_address, IF(at.member_id IS NULL, 'No', 'Yes') as 'present', IF(ab.member_id IS NULL, 'No', 'Yes') as 'absent' FROM members m LEFT JOIN attendance at ON at.member_id = m.id LEFT JOIN absence ab ON ab.member_id = m.id WHERE m.precinct = '01-07' AND (at.present = 1 OR ab.absent = 1) ORDER BY m.precinct, m.id;";
		$stmt = $pdo->prepare($sql);
		$stmt->execute();
		while($data = $stmt->fetch(PDO::FETCH_OBJ)) {
			$x = $this->GetX();
			$this->SetX(21);
			$this->Cell(20, 10, $data->precinct, 1, 0, 'C');
			$this->Cell(60, 10, $data->full_name, 1, 0, 'C');
			$this->Cell(100, 10, $data->residential_address, 1, 0, 'C');
			$this->Cell(20, 10, $data->present, 1, 0, 'C');
			$this->Cell(36, 10, $data->absent, 1, 0, 'C');
			$this->Ln();
			$this->SetX($x);		
		}
	}
}

$pdf = new Report();
$pdf->SetTitle('CABGOP | Precinct Report');
$pdf->AliasNbPages();
$pdf->AddPage('L', 'Letter', 0);
$pdf->headerPrecinct();
$pdf->viewPrecinct_0102();
$pdf->AddPage('L', 'Letter', 0);
$pdf->headerPrecinct();
$pdf->viewPrecinct_0104();
$pdf->AddPage('L', 'Letter', 0);
$pdf->headerPrecinct();
$pdf->viewPrecinct_0107();
$pdf->Output();

?>

with this query:

SELECT precinct, 
CONCAT(last_name, ', ', first_name, ' ', middle_name, ' ', IFNULL(suffix, ' ')) as full_name, residential_address, 
IF(at.member_id IS NULL, 'No', 'Yes') as 'present', 
IF(ab.member_id IS NULL, 'No', 'Yes') as 'absent'
FROM members m 
LEFT JOIN attendance at ON at.member_id = m.id 
LEFT JOIN absence ab ON ab.member_id = m.id 
WHERE m.precinct = '01-02' AND (at.present = 1 OR ab.absent = 1) 
ORDER BY m.precinct, m.id;

and get the same result?

For the record, I am using FPDF to generate this report. I cannot post the entire PDF code as I am limited to how long my post can be, but basically, the viewPrecinct() function repeats throughout the entire code and I have to list $pdf->viewPrecinct() at the end numerous times.

You need to use a datacentric approach, rather than a piecemeal approach. Query to get all the data that you want in the order that you want it, index/pivot the data using the precinct number when you retrieve it, then simple loop over the resultant arrays of arrays of data, supplying each array of precinct data to a general-purpose function that produces the output from that data. This will also separate the database specific code, that knows how to query for and retrieve the data, from the presentation code, that knows how to produce the desired output.

This query:

SELECT precinct, 
CONCAT(last_name, ', ', first_name, ' ', middle_name, ' ', IFNULL(suffix, ' ')) as full_name, residential_address, 
IF(at.member_id IS NULL, 'No', 'Yes') as 'present', 
IF(ab.member_id IS NULL, 'No', 'Yes') as 'absent'
FROM members m 
LEFT JOIN attendance at ON at.member_id = m.id 
LEFT JOIN absence ab ON ab.member_id = m.id 
WHERE at.present = 1 OR ab.absent = 1 
ORDER BY m.precinct, m.id

shows all of the data I need, but I would like precincts to be separated by page. Forgive me, but I don’t quite understand. Can you explain a bit further by giving an example?

This

function viewPrecinct_0102() {
... 
function viewPrecinct_0104() {

seems as if it could be replaced by

function viewPrecinct($precinct_code) {

and pass the code into the function. And maybe this

$pdf->AddPage('L', 'Letter', 0);
$pdf->headerPrecinct();
$pdf->viewPrecinct_0102();
$pdf->AddPage('L', 'Letter', 0);
$pdf->headerPrecinct();
$pdf->viewPrecinct_0104();

could loop through a SELECT DISTINCT result set to build a list of precincts, rather than hard-coding it. And maybe your view_precinct() function should call headerPrecinct() rather than the calling code doing it each time?

Equally, though, could you run a single query, and when your precinct changes, throw a new page and a header?

// pseudo-code
run query
loop through results:
  if new-precinct <> last-precinct
    output footer for last-precinct
    throw page 
    output header for new precinct
    end if
  output data
  last-precinct = new-precinct
end of loop

That was one of my thoughts in rewriting it, but I am not sure what the best way to define the precinct would be? Based on the query I provided in my last reply, I get this in the precinct column:

image

There’s multiple values with the same precinct. I want that to show in the data that returns in the table.

I researched SELECT DISTINCT and from what I read, DISTINCT is used to select different data rather than data with duplicate values. Are you saying that I would be using 2 queries in this report? I don’t see how SELECT DISTINCT would change the data that is generated.

That is a possibility, but I need to add another header in here that shows what page is what precinct. For example, one page would say “Precinct 01-01” the next “Precinct 01-02” and so on.

Yes, but what happens to the data?

The same way that you do it now, but by retrieving it from the database rather than hard-coding it.

Yes, instead of your hard-coded called to each function, you’d use a query to get a list of all the precincts that are in your data, and SELECT DISTINCT would ensure each one appeared only once. You then loop through those results calling your viewprecinct function for each precinct.

I ran this query SELECT DISTINCT precinct FROM members and got this:

image

So now you mention to

So, if I understand that correctly, that entire function is a loop. But what about my other query that grabs all of the data I need to view in the report? How does the loop take care of that?

Here is my current viewPrecinct() function after working on this for some time:

function viewPrecinct() {
		global $pdo;
		$this->SetFont('Arial', '', 8);
		$precinct_query = "SELECT DISTINCT precinct FROM members";
		$stmt = $pdo->prepare($precinct_query);
		$stmt->execute();
		$result = $stmt->fetchAll(PDO::FETCH_ASSOC);
		foreach($result as $precinct) {
			$x = $this->GetX();
			$this->SetX(21);
			$this->SetFont('Arial', 'B', 10);
			$this->Cell(20, 10, 'Precinct', 1, 0, 'C');
			$this->Cell(60, 10, 'Member Name', 1, 0, 'C');
			$this->Cell(100, 10, 'Residential Address', 1, 0, 'C');
			$this->Cell(20, 10, 'Present', 1, 0, 'C');
			$this->Cell(36, 10, 'Excused Absence', 1, 0, 'C');
			$this->Ln();
			$this->SetX($x);
			
			implode(" ", $precinct) . "<br>";
			$this->AddPage('L', 'Letter', 0);
			$query_sql = "SELECT precinct, CONCAT(last_name, ', ', first_name, ' ', middle_name, ' ', IFNULL(suffix, ' ')) as full_name, residential_address, IF(at.member_id IS NULL, 'No', 'Yes') as 'present', IF(ab.member_id IS NULL, 'No', 'Yes') as 'absent' FROM members m LEFT JOIN attendance at ON at.member_id = m.id LEFT JOIN absence ab ON ab.member_id = m.id WHERE m.precinct = '12-13' AND (at.present = 1 OR ab.absent = 1) ORDER BY m.precinct, m.id;";
			$stmt = $pdo->prepare($query_sql);
			$stmt->execute();
			while($data = $stmt->fetchAll(PDO::FETCH_OBJ)) {
				$x = $this->GetX();
				$this->SetX(21);
				$this->Cell(20, 10, $data->precinct, 1, 0, 'C');
				$this->Cell(60, 10, $data->full_name, 1, 0, 'C');
				$this->Cell(100, 10, $data->residential_address, 1, 0, 'C');
				$this->Cell(20, 10, $data->present, 1, 0, 'C');
				$this->Cell(36, 10, $data->absent, 1, 0, 'C');
				$this->Ln();
				$this->SetX($x);			
			} 
		}
	} 

The header appears, but there is no data…?

No, I meant that you would call your viewPrecinct function from a loop, and pass in the precinct. It was just a way for you to not hard-code each call.

// main code
$this->SetFont('Arial', '', 8);
$precinct_query = "SELECT DISTINCT precinct FROM members";
$stmt = $pdo->prepare($precinct_query);
$stmt->execute();
$result = $stmt->fetchAll(PDO::FETCH_ASSOC);
foreach($result as $precinct) {
    $pdf->AddPage('L', 'Letter', 0);
    $pdf->headerPrecinct();
    $pdf->viewPrecinct($precinct, $pdo); // not sure if this is the correct column name
    }

and then you change viewPrecinct() to use the precinct id that you pass into the function. While you’re at it, pass in the $pdo variable as well - global variables aren’t really a nice thing.

Do NOT run SELECT queries inside of loops. This is extremely inefficient.

The 1st reply in this thread gave a simple and efficient method to solve this.

The OP seemed reluctant to do a single query and deal with page breaks in there, that’s what I was trying to outline in my pseudo-code earlier (in a sort-of expansion to your post, as a sort-of example that the OP requested), so I figured it might be a step in the right direction to at least help improve what they already had.

Okay, now I’m confused…

This is my current headerPrecinct()function that displays the table header.

function headerPrecinct() {
		$x = $this->GetX();
		$this->SetX(21);
		$this->SetFont('Arial', 'B', 10);
		$this->Cell(20, 10, 'Precinct', 1, 0, 'C');
		$this->Cell(60, 10, 'Member Name', 1, 0, 'C');
		$this->Cell(100, 10, 'Residential Address', 1, 0, 'C');
		$this->Cell(20, 10, 'Present', 1, 0, 'C');
		$this->Cell(36, 10, 'Excused Absence', 1, 0, 'C');
		$this->Ln();
		$this->SetX($x);
		
	}

This is the viewPrecinct() function that shows the data in the table:

function viewPrecinct() {
		$this->SetFont('Arial', '', 8);
		$sql = "SELECT precinct, CONCAT(last_name, ', ', first_name, ' ', middle_name, ' ', IFNULL(suffix, ' ')) as full_name, residential_address, IF(at.member_id IS NULL, 'No', 'Yes') as 'present', IF(ab.member_id IS NULL, 'No', 'Yes') as 'absent' FROM members m LEFT JOIN attendance at ON at.member_id = m.id LEFT JOIN absence ab ON ab.member_id = m.id WHERE m.precinct = '01-02' AND (at.present = 1 OR ab.absent = 1) ORDER BY m.precinct, m.id;";
		$stmt = $pdo->prepare($sql);
		$stmt->execute();
		while($data = $stmt->fetch(PDO::FETCH_OBJ)) {
			$x = $this->GetX();
			$this->SetX(21);
			$this->Cell(20, 10, $data->precinct, 1, 0, 'C');
			$this->Cell(60, 10, $data->full_name, 1, 0, 'C');
			$this->Cell(100, 10, $data->residential_address, 1, 0, 'C');
			$this->Cell(20, 10, $data->present, 1, 0, 'C');
			$this->Cell(36, 10, $data->absent, 1, 0, 'C');
			$this->Ln();
			$this->SetX($x);			
		}
	}

This snip of code:

$this->SetFont('Arial', '', 8);
$precinct_query = "SELECT DISTINCT precinct FROM members";
$stmt = $pdo->prepare($precinct_query);
$stmt->execute();
$result = $stmt->fetchAll(PDO::FETCH_ASSOC);
foreach($result as $precinct) {
    $pdf->AddPage('L', 'Letter', 0);
    $pdf->headerPrecinct();
    }

worked before when I tried to make a new page for each precinct. Although, based on what you are describing, it looks that the code could be another function I could add?

I can’t place the foreach loop at the end since that breaks the code, so that has to be part of a function. Am I adding this code to the headerPrecinct() or viewPrecinct() function, or am I making a new function (maybe called Output()?)

Doing it this way, leaving aside the point that @mabismad made earlier, your overall code would look like this:

select distinct query to get a list of precincts
for each precinct: 
   add a new page
   output a page header
   output the details for that precinct

If it has to be in a function which calls the other functions, then that’s fine.

I think the point @mabismad was making is that you can do a single query for this, rather than a query to get precincts, and another query for each precinct. So as you started (I think) with a query that recovers all the results in a single query, ordered by precinct, you could go back to my pseudo-code earlier:

run the main query, and get all the results
for each result:
   has the precinct changed since the previous result?
     if yes: throw a page header and output the table headers
   output the line of information

and that’s it. You run a single query, loop through the results, add in your page and table headers any time the precinct changes, and it’s done.

(Sorry I use pseudo-code instead of actual PHP - I am not sufficiently proficient in PHP to not make syntax errors without looking stuff up, and there’s nothing more annoying than giving examples that don’t work. In any case, the actual coding is your task, I’m just talking about the design of the code).

Yes, I understand. I was getting confused between the responses trying to make sense of them.

The code mentioned earlier I placed in another function (as it’s really the only way it makes sense to me):

function viewAll() {
		$this->SetFont('Arial', '', 8);
		$precinct_query = "SELECT DISTINCT precinct FROM members";
		$stmt = $pdo->prepare($precinct_query);
		$stmt->execute();
		$result = $stmt->fetchAll(PDO::FETCH_ASSOC);
		foreach($result as $precinct) {
	    	$pdf->AddPage('L', 'Letter', 0);
    		$pdf->headerPrecinct();
			$pdf->viewPrecinct();
    	}
	}

There’s only one problem. You said

I can’t use the $pdo variable unless I use

global $pdo

because the line that contains this variable is an include statement to my database connector which is not contained in a function.

include("../dbconnect.php");

If there is a way of removing the global variable, how would I do it? I tried removing it before, but the code came back with an error that said there was too many arguments in the function.

Also, I’m getting the same problem as before: one blank page with just the PDF header, and the rest have table headers with no data on any of the pages.

At this point, you’d use

$pdf->viewPrecinct($precinct);

and change the function definition to have that parameter, and then use the precinct you pass in with that parameter in your query.

I think the issue is that all your code is contained within a class, and you don’t pass in $pdo to that class. I don’t really do much with OO stuff, but could you declare another private variable inside your class, and pass $pdo into it just after you’ve created the class with a setDB() function? For now, if it works just leave it, but while (in my opinion) it’s a Bad Thing to use a global variable inside a function, it’s even worse (in my opinion) to use it inside a class. The whole point of this structured stuff is that you can use it anywhere, so it can’t rely on outside stuff.

What does the code look like now? I notice your latest viewPrecinct() function is still hard-coding the precinct in the query.

(Note that when I use $precinct in my call to viewPrecinct(), I’m presuming that contains the correct value which changes each time you loop).

Yes, the PDF report begins with this:

class Report extends FPDF {
	function header() {
		$this->Image('../../img/ccrp_seal.png', 10, 6, 25, 25);
		$this->SetFont('Arial', 'B', 14);
		$this->Cell(275, 5, 'Cabarrus County GOP Convention' . ' ' . date("Y"), 0, 0, 'C');
		$this->Ln();
		$this->SetFont('Arial', '', 12);
		$this->Cell(275, 10, 'Precinct Report', 0, 0, 'C');
		$this->Ln(25);
	}

Here is the code at the moment that shows all 3 functions:

function headerPrecinct() {
		$x = $this->GetX();
		$this->SetX(21);
		$this->SetFont('Arial', 'B', 10);
		$this->Cell(20, 10, 'Precinct', 1, 0, 'C');
		$this->Cell(60, 10, 'Member Name', 1, 0, 'C');
		$this->Cell(100, 10, 'Residential Address', 1, 0, 'C');
		$this->Cell(20, 10, 'Present', 1, 0, 'C');
		$this->Cell(36, 10, 'Excused Absence', 1, 0, 'C');
		$this->Ln();
		$this->SetX($x);
		
	}
	
	function viewPrecinct() {
		global $pdo;
		$this->SetFont('Arial', '', 8);
		$sql = "SELECT precinct, CONCAT(last_name, ', ', first_name, ' ', middle_name, ' ', IFNULL(suffix, ' ')) as full_name, residential_address, IF(at.member_id IS NULL, 'No', 'Yes') as 'present', IF(ab.member_id IS NULL, 'No', 'Yes') as 'absent' FROM members m LEFT JOIN attendance at ON at.member_id = m.id LEFT JOIN absence ab ON ab.member_id = m.id WHERE m.precinct = '01-02' AND (at.present = 1 OR ab.absent = 1) ORDER BY m.precinct, m.id;";
		$stmt = $pdo->prepare($sql);
		$stmt->execute();
		while($data = $stmt->fetch(PDO::FETCH_OBJ)) {
			$x = $this->GetX();
			$this->SetX(21);
			$this->Cell(20, 10, $data->precinct, 1, 0, 'C');
			$this->Cell(60, 10, $data->full_name, 1, 0, 'C');
			$this->Cell(100, 10, $data->residential_address, 1, 0, 'C');
			$this->Cell(20, 10, $data->present, 1, 0, 'C');
			$this->Cell(36, 10, $data->absent, 1, 0, 'C');
			$this->Ln();
			$this->SetX($x);			
		}
	}
	
	function viewAll() {
		global $pdo;
		$this->SetFont('Arial', '', 8);
		$sql = "SELECT DISTINCT precinct FROM members";
		$stmt = $pdo->prepare($sql);
		$stmt->execute();
		$result = $stmt->fetchAll(PDO::FETCH_ASSOC);
		foreach($result as $precinct) {
	    	$this->AddPage('L', 'Letter', 0);
    		$this->headerPrecinct();
			$this->viewPrecinct($precinct);
    	}
	}

UPDATE TO PREVIOUS REPLY

I changed the SQL statement from this:

$sql = "SELECT precinct, 
CONCAT(last_name, ', ', first_name, ' ', middle_name, ' ', IFNULL(suffix, ' ')) as full_name, residential_address, 
IF(at.member_id IS NULL, 'No', 'Yes') as 'present', 
IF(ab.member_id IS NULL, 'No', 'Yes') as 'absent' 
FROM members m 
LEFT JOIN attendance at ON at.member_id = m.id 
LEFT JOIN absence ab ON ab.member_id = m.id 
WHERE m.precinct = '01-02' AND (at.present = 1 OR ab.absent = 1) 
ORDER BY m.precinct, m.id;";

to this:

$sql = "SELECT precinct, 
CONCAT(last_name, ', ', first_name, ' ', middle_name, ' ', IFNULL(suffix, ' ')) as full_name, residential_address, 
IF(at.member_id IS NULL, 'No', 'Yes') as 'present', 
IF(ab.member_id IS NULL, 'No', 'Yes') as 'absent' 
FROM members m LEFT JOIN attendance at ON at.member_id = m.id 
LEFT JOIN absence ab ON ab.member_id = m.id 
WHERE at.present = 1 OR ab.absent = 1 
ORDER BY m.precinct, m.id;";

I can see data now, but the data is not separated by precinct on each page like I need it to be.
I don’t understand what I need to do to get the data split into separate precincts on each page.

Basically, if Precinct 01-02 has no data, keep the page blank. If it does have data, then display the data only for that precinct. Then for precinct 01-04, create a new page and repeat the process. It sounds like I’m overthinking this, but it doesn’t make sense to me. How can I seperate the data without hardcoding the precinct in the SQL statement? Where does the $precinct get it’s value?

Hang on a second - do you need a page for a precinct, even if that precinct doesn’t appear in your query results, so you’d have a page with just a header? That’s quite a different thing - presumably you have a “precincts” table that contains all precincts?

By comparing the precinct value in the “current” row with the precinct value in the “previous” row - as soon as it changes, you know the report has moved on to another precinct, and at that point you need to throw a new page and draw the header again.

From your foreach through the results of your queries.