Topic: Don't use globals? But how else can I do this?

[V]

Almost every time when I read about globals, programmers discourage their use. I have a function where I need to send back some variables (which I will use in a query) so I need to use globals. Below is the function and query. I'm trying to figure out if it's ok to use globals in it.

function paginate($connection, $tableName) {
//the forsaken globals
global $limit; 
global $start;

//Pagination
$targetpage = "http://localhost/website/untitled2.php"; 

$limit = 4; 

//count rows
$sql = "SELECT COUNT(*) as num FROM $tableName";
$total_pages = $connection->query($sql) or die(mysqli_error($connection)); 
$row = $total_pages->fetch_assoc();
$total_pages = $row['num'];

//if there's no page number, set it to the first page
$stages = 3;
$page = isset($_GET['page']) ? $_GET['page'] : 0;
$start = empty($page) ? 0 : ($page - 1) * $limit;

// Initial page num setup

if ($page == 0){$page = 1;}

$prev = $page - 1;

$next = $page + 1;

$lastpage = ceil($total_pages/$limit);

$LastPagem1 = $lastpage - 1;

$paginate = '';

if($lastpage > 1)

{

$paginate .= "<div class='paginate'>";

// Previous

if ($page > 1){

$paginate.= "<a href='$targetpage?page=$prev'>previous</a>";

}else{

$paginate.= "<span class='disabled'>previous</span>";

}

// Pages

if ($lastpage < 7 + ($stages * 2))

// Not enough pages to breaking it up

{

for ($counter = 1; $counter <= $lastpage; $counter++)

{

if ($counter == $page){

$paginate.= "<span class='current'>$counter</span>";

}else{

$paginate.= "<a href='$targetpage?page=$counter'>$counter</a>";}

}

}

elseif($lastpage > 5 + ($stages * 2))

// Enough pages to hide a few?

{

// Beginning only hide later pages

if($page < 1 + ($stages * 2))

{

for ($counter = 1; $counter < 4 + ($stages * 2); $counter++)

{

if ($counter == $page){

$paginate.= "<span class='current'>$counter</span>";

}else{

$paginate.= "<a href='$targetpage?page=$counter'>$counter</a>";}

}

$paginate.= "...";

$paginate.= "<a href='$targetpage?page=$LastPagem1'>$LastPagem1</a>";

$paginate.= "<a href='$targetpage?page=$lastpage'>$lastpage</a>";

}

// Middle hide some front and some back

elseif($lastpage - ($stages * 2) > $page && $page > ($stages * 2))

{

$paginate.= "<a href='$targetpage?page=1'>1</a>";

$paginate.= "<a href='$targetpage?page=2'>2</a>";

$paginate.= "...";

for ($counter = $page - $stages; $counter <= $page + $stages; $counter++)

{

if ($counter == $page){

$paginate.= "<span class='current'>$counter</span>";

}else{

$paginate.= "<a href='$targetpage?page=$counter'>$counter</a>";}

}

$paginate.= "...";

$paginate.= "<a href='$targetpage?page=$LastPagem1'>$LastPagem1</a>";

$paginate.= "<a href='$targetpage?page=$lastpage'>$lastpage</a>";

}

// End only hide early pages

else

{

$paginate.= "<a href='$targetpage?page=1'>1</a>";

$paginate.= "<a href='$targetpage?page=2'>2</a>";

$paginate.= "...";

for ($counter = $lastpage - (2 + ($stages * 2)); $counter <= $lastpage; $counter++)

{

if ($counter == $page){

$paginate.= "<span class='current'>$counter</span>";

}else{

$paginate.= "<a href='$targetpage?page=$counter'>$counter</a>";}

}

}

}

// Next

if ($page < $counter - 1){ 

$paginate.= "<a href='$targetpage?page=$next'>next</a>";

}else{

$paginate.= "<span class='disabled'>next</span>";

}

$paginate.= "</div>";

}
 echo $total_pages.' Results';
 // pagination
 echo $paginate;
 
}//end function


and this is how I'm using the function. Without the globals I would get undefined vars $start and $limit used in the query below.


paginate($connection, "categories"); 

$sql = "SELECT * FROM categories ORDER BY cat_name LIMIT $start, $limit";
$cats_result = $connection->query($sql) or die(mysqli_error($connection)); 
while ($row = $cats_result->fetch_assoc()) { 

$cat_id = $row['cat_id'];

$cat_name = $row['cat_name'];

$cat_desc = $row['cat_desc'];

...etc


Am I using the globals properly? 

[litebearer]

Perhaps consider using session variables???

[Joshua4550]

That code works, so if you're fine with when they're setting.. I'd say it's fine to use them that way.

[AlexWD]

The whole thing is designed poorly to be honest. It would be a better idea to construct variables like $start and $limit outside of the function and make the pagination() function accept only what it needs in order to do its purpose. All of this stuff:

$sql = "SELECT COUNT(*) as num FROM $tableName";
$total_pages = $connection->query($sql) or die(mysqli_error($connection)); 
$row = $total_pages->fetch_assoc();
$total_pages = $row['num'];

//if there's no page number, set it to the first page
$stages = 3;
$page = isset($_GET['page']) ? $_GET['page'] : 0;
$start = empty($page) ? 0 : ($page - 1) * $limit;

Should be done outside of the function. Then you can pass what the function needs to do its job. Logically a pagination function shouldn't have to worry about where the data is coming from. In your case it's bound strictly to a mysqli setup. If you wanted to reuse this function in another project later that didn't use mysqli you would have to rewrite the function. A function should have a single task; it shouldn't have to fetch the data itself and create the pagination links. By separating concerns in a way that a function only has a single task you write more flexible code, and that should always be your objective.

[V]

AlexWD I was actually considering that but I wasn't sure. I went ahead and did it and it is indeed much better Thank you! Last thing I should do is put all the vars into an array to pass just 1 var into the function arguments instead of

function paginateBottom($page, $limit, $total_pages, $stages, $targetpage)

[AlexWD]

There's really no point in doing that.

[V]

You mean the array?

function paginateBottom($page, $limit, $total_pages, $stages, $targetpage)

works fine

[AlexWD]

Yeah, that's fine. I thought you were suggesting that you should put them into an array.

[V]

Oh I wanted to try something to make the function shorter and insert all those variables via 1 variable (perhaps an array containing them all) but I guess it's just an aesthetic thing.