1
\$\begingroup\$

Am I doing this right? I do not know how to properly connect functions with pagination. To make everything work, I have to duplicate this variables. When I add variables to the function and to the index, then everything works.

$rowperpage = 10; 
$page = $_GET['page'] ?? 1;
$page = $page - 1;      
$p = $page * $rowperpage;

Function

function find_all_products_by_cat_id2($cat_id, $options=[]) {
    global $db;

    $visible = $options['visible'] ?? false;

    $rowperpage = 10;
    $page = $_GET['page'] ?? 1;
    $page = $page - 1;
    $p = $page * $rowperpage;

    $sql = "SELECT * FROM products ";
    $sql .= "WHERE cat_id='" . db_escape($db, $cat_id) . "' ";  
    if($visible) {
      $sql .= "AND visible = true ";
    }
    $sql .= "ORDER BY prod_name ASC ";
    $sql .= "LIMIT ".$p.", ".$rowperpage." ";
    $result = mysqli_query($db, $sql);
    confirm_result_set($result);
    return $result;

index.php

<?php 

        $rowperpage = 10;
        $page = $_GET['page'] ?? 1;
        $page = $page - 1;
        $p = $page * $rowperpage;

        $category_id = $_GET['id'] ?? 1;

        $products_count = count_products_by_cat_id($category_id, ['visible' => true]);


        if(isset($_GET['id'])) {
        $category_id = $_GET['id'];

        $product_set = find_all_products_by_cat_id2($category_id, ['visible' => true]);

        if(!$product_set) {
            redirect_to(url_for('/index.php'));
        }

        $product_count = mysqli_num_rows($product_set);

        if($product_count == 0) {
              echo "<h1>No more products</h1>";
              }

 ?>     

I think I found a solution.

Update 1
Function

  function find_by_sql($sql) {
    global $db;

    $result = mysqli_query($db, $sql);
    confirm_result_set($result);
    return $result;
  } 

index.php

$visible = $options['visible'] ?? false;

$sql = "SELECT * FROM products ";
$sql .= "WHERE cat_id='" . db_escape($db, $category_id) . "' "; 
if($visible) {
  $sql .= "AND visible = true ";
}
$sql .= "ORDER BY prod_name ASC ";
$sql .= "LIMIT ".$p.", ".$rowperpage." "; 

$product_set = find_by_sql($sql, ['visible' => true]);
\$\endgroup\$
  • \$\begingroup\$ "Am I doing this right?" Most importantly, does it work? You say it works when you add variables to the function and the index, does that suffice for your application? Sure, you're looking for something better, but the current code does at least the bare minimum of what is required? \$\endgroup\$ – Mast Mar 15 at 11:07
  • \$\begingroup\$ Its work, I think I found a solution. Check please Update1 \$\endgroup\$ – Mantykora 7 Mar 15 at 11:23
2
\$\begingroup\$

Assuming you have a BOOL/BOOLEAN column called visible, you might consider using a more recognized syntax like visible <> 0 or visible = 0 as advised here: MySQL: “= true” vs “is true” on BOOLEAN. When is it advisable to use which one? And Which one is vendor independent? After all, BOOLEAN columns are really just TINYINT columns.

I have to assume that confirm_result_set($result); is fetching the rows and preparing them in some manner. Perhaps you'd like to adjust the variable name to be more intuitive/descriptive.

Some developers, myself included, do not recommend the use of global as a means of transferring variables into a function's scope. IMO, it is cleaner to pass the connection variable as a parameter in the function call (like you do with $sql).

I don't know what db_escape() is doing, but the surest advice is to urge you to use prepared statements with placeholders. As a less classy alternative, so long as $category_id is an integer, you can cast it as an integer ((int)$category_id) before using it in your query. Joomla, for instance, doesn't offer prepared statements yet (available in the next major version) so they use integer/float casting for security on numeric values. Also, numeric values do not need to be quoted in queries.

ASC is not necessary in your query, it is the default sorting direction and can be omitted.

The LIMIT clause string can be written without concatenation: $sql .= "LIMIT $p, $rowperpage";

\$\endgroup\$
  • \$\begingroup\$ function db_escape($connection, $string) { return mysqli_real_escape_string($connection, $string); } \$\endgroup\$ – Mantykora 7 Apr 9 at 8:49

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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