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.

[Alex]

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)

[Alex]

There's really no point in doing that.

[V]

You mean the array?

 

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

 

works fine

[Alex]

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.