# Paginated method to get all items

I'm using slim php and i have this simple paginated end point to get all clients, i feel like i could do some things here better, so i would appreciate all suggestions, if possible please give me code example with suggested improvements. Thank you in advance.

$app->get('/api/clients', function ($request, $response,$args) {
require_once 'dbConnect.php';
require_once 'shared/securityService.php';
$queryParams =$request->getQueryParams();

$total = getTotalCount($mysqli, $queryParams["group"]);$pageSize = $queryParams["pageSize"];$totalPages = ceil($total /$pageSize);
$offset = ($queryParams["page"] - 1)  * $pageSize; if(is_numeric($queryParams["group"])) {
$stmt =$mysqli->prepare("SELECT id, firstName, lastName, birthDate, phoneNumber, address,
contactPerson, contactPersonPhoneNumber FROM client WHERE groupId = ? ORDER BY
id LIMIT $pageSize OFFSET$offset");

$stmt->bind_param("i",$queryParams["group"]);
} else {
$stmt =$mysqli->prepare("SELECT id, firstName, lastName, birthDate, phoneNumber, address,
contactPerson, contactPersonPhoneNumber FROM client
ORDER BY id LIMIT $pageSize OFFSET$offset");
}

$stmt->execute();$stmt->bind_result($id,$firstName, $lastName,$birthDate, $phoneNumber,$address, $contactPerson,$contactPersonPhoneNumber);

$data = null; while ($stmt->fetch()) {
$data['items'][] = array( 'id' =>$id,
'firstName' => $firstName, 'lastName' =>$lastName,
'birthDate' => $birthDate, 'phoneNumber' =>$phoneNumber,
'address' => $address, 'contactPerson' =>$contactPerson,
'contactPersonPhoneNumber' => $contactPersonPhoneNumber ); }$data['totalItems'] = $total;$stmt->close();
$mysqli->close(); echo json_encode($data);
});

function getTotalCount($mysqli,$groupId) {
if(is_numeric($groupId)) {$totalStmt = $mysqli->prepare("SELECT COUNT(id) FROM client WHERE groupId = ?");$totalStmt->bind_param("i", $groupId); } else {$totalStmt = $mysqli->prepare("SELECT COUNT(id) FROM client"); }$totalStmt->execute();
$totalStmt->bind_result($total);
$totalStmt->fetch();$totalStmt->close();
return $total; }  ## 2 Answers SQL Injection You are vulnerable to SQL injection via pageSize. You need to use prepared statements for all values, not just some of them. Naming total and getTotalCount could be more descriptive. Currently, I don't really know what "total" means without looking at the code of the function. You also don't need "total" at all (it's assumed when talking about a count). $clientCount = getClientCountForGroup($mysqli,$id) would be easier to understand.

Duplication

Your two queries are basically duplicates apart from the WHERE clause. You may want to remove that duplication.

• Thx for suggestions, what is best way to remove duplication ? Mar 15 '17 at 19:27

PDO

Like I said before, moving to PDO will reduce the following code block

$stmt->bind_result($id, $firstName,$lastName, $birthDate,$phoneNumber,
$address,$contactPerson, $contactPersonPhoneNumber);$data = null;

while ($stmt->fetch()) {$data['items'][] = array(
'id' => $id, 'firstName' =>$firstName,
'lastName' => $lastName, 'birthDate' =>$birthDate,
'phoneNumber' => $phoneNumber, 'address' =>$address,
'contactPerson' => $contactPerson, 'contactPersonPhoneNumber' =>$contactPersonPhoneNumber
);
}


to

$data['items'] =$stmt->fetchAll();


Programming

It's always a good idea to create a function for the repetitive task. For running a query for example - an action that takes preparation, binding, execution, results retrieval. A function that takes a query and parameters as its arguments and returns the resulting data is always useful. A class would be better than a function though.

For example, it will make something like

function getClientCount($db,$groupId = 0) {
if($groupId) {$stmt= $db->run("SELECT COUNT(id) FROM client WHERE groupId = ?",[$groupId]);
} else {
$stmt =$db->run("SELECT COUNT(id) FROM client");
}
return \$stmt->fetchColumn();
}