Jump to content

Recommended Posts

I am getting the exact same error over and over again, and I have no idea why:

 

Quote

PHP Warning: Undefined variable $total in C:\inetpub\wwwroot\pages\poll\includes\functions.php on line 27

Consider the code

 

function calculateResults($pollObj) {
		$conn = null;
		$stmt = null;
		$rs = null;
		$total = 0;
		global $resultsCalcArray;
		try {
			$pollId = stripHTML(cleanXSS($pollObj->id));
			$conn = new PDO(DB_CONNECTION_STR, MYSQL_DB_USER, MYSQL_DB_PASSWORD, MYSQL_DB_PDO_OPTIONS);
			$stmt = $conn->prepare(RESULTS_SQL);
			$stmt->execute([$pollId, $pollId, $pollId]);
			$rs = $stmt->fetchAll(PDO::FETCH_ASSOC);
			if (!is_null($rs)) {
				foreach ($rs as $row) {
					if (!empty($row['kount'])) {
						$total += (int) $row['kount'];
						array_push($resultsCalcArray, $row['kount']);
					}
				}
			}
			
			$_SESSION['total'] = $total;
			if ($total > 0) {							// TO PREVENT DIVIDE BY ZERO ERROR
				$resultsCalcArray = array_map(function($votes) {
												 return round($votes / $total) * 100;
											  }, $resultsCalcArray);
			}
			
		} catch (Exception $e) {
			$msg = ERROR_MESSAGE . ' calculateResults() ' . date('Y-m-d H:i:s') . ' ' . $e->getMessage();
			toLogDB($msg);
			error_log($msg, 0);						
			throw $e;
			$hasErrors = true;
		} finally {
			if (!is_null($rs)) {
				$rs = null;
			}
			 
			if (!is_null($stmt)) {
				$stmt = null;
			}
									
			if (!is_null($conn)) {
				$conn = null;
			}
		}
	}

I honestly don't know what I did wrong here, but it is completely failing the entire code within the function inside array_map(), and I have no idea why.

Help appreciated and needed.

Thanks

To include a variable that is defined externally to the map function you need "use()"

    $calcArray = [ 34, 56, 82 ];
    
    $total = 10;
    
    $calcArray = array_map (
                    function($v) use ($total) {
                        return round($v / $total);
                    },
                    $calcArray
                 );
                 
    echo '<pre>' . print_r($calcArray, 1) . '</pre>';

Alternatively, you can use this syntax...

    $calcArray = [ 34, 56, 82 ];
    
    $total = 10;
    
    $calcArray = array_map (
                    fn($v) => round($v / $total),
                    $calcArray
                 );
                 
    echo '<pre>' . print_r($calcArray, 1) . '</pre>';

See https://www.php.net/manual/en/functions.arrow.php

  • Great Answer 1

the anonymous function has local variable scope, like any php function. you can add use ($total) to the definition to make the variable available inside the function -

$resultsCalcArray = array_map(function($votes) use ($total) {

 

as to the posted code -

  1. the input call-time parameter should only be the $id and you should only call this function after you have validated the $id.
  2. this function should also have an existing pdo connection as an input call-time parameter. it is the wrong responsibility for this function to make a database connection.
  3. there's no point is defining and initializing $conn, $stmt, and $rs.
  4. don't use the global keyword to get data into or out of a function. this is not general purpose and results in spaghetti code that is hard to debug problems in. all input data to a function should be supplied as call-time parameters and the function should return the result it produces to the calling code.
  5. if you set the default fetch mode to assoc when you make the database connection, you won't need to specify it in each fetch statement.
  6. fetchAll() won't ever return a null, so the !is_null() test will never fail and should be removed.
  7. since you are only operating on the 'kount' column, that is the only thing the query should SELECT.
  8. if you have some reason to select other columns, you can build (or add to) the $resultsCalcArray from just the 'kount' column by using array_column().
  9. you can directly get the total of the $resultsCalcArray 'kount' values by using array_sum().
  10. no in-line code after the throw $e; statement will ever get executed and should be removed.
  11. there's generally no point in freeing up result sets, closing prepared query handles, or closing database connections in your code, at all, and there's certainly no point in doing this inside a function, since all those things get destroyed when the function call ends.
  • Great Answer 1
5 hours ago, mac_gyver said:

 

as to the posted code -

 ... this function should also have an existing pdo connection as an input call-time parameter. it is the wrong responsibility for this function to make a database connection.

 ... there's no point is defining and initializing $conn, $stmt, and $rs.

 

The general principles involved in mac_gyver's valuable comments to you, when applied to classes, include the idea of dependency injection (aka "inversion of control").

If you consider your function, what are the dependencies?

The first thing to look for would be objects you are creating internally using new keyword.  In your code that is this line:

 

$conn = new PDO(DB_CONNECTION_STR, MYSQL_DB_USER, MYSQL_DB_PASSWORD, MYSQL_DB_PDO_OPTIONS);

Rather than creating objects inside the function, you should pass them as parameters.

Objects and resources are automatically passed by reference, so you are able to use the class inside the function without limitation, including calling methods that mutate the object.

 

Here's a short video that explains Dependency Injection further.

 

While Dependency injection is an OOP specific design pattern, the philosophy can still be applied to your functional code in most cases.  You won't have a DI Container, but you can work around that as long as you understand the idea.

So to begin to address many of the things brought up by mac_gyver, I'd expect your function signature to look more like this:

function calculateResults(PDO $conn, Poll $pollObj, Array &$resultsCalcArray) {
   // implementation
}

 

You are also using the $_SESSION superglobal.  A strong argument can be made that you should also pass the superglobal into your function, rather than simply relying on it's superglobal property.  Most frameworks have their own session wrapping class, but you could simply use a parameter like $session, and pass $_SESSION into your function.  One reason to do that, is that you can then create unit tests for your function, which would not be possible normally, because $_SESSION only exists in the web context and doesn't exist in the CLI environment.

So an alternative would be to do this:

function calculateResults(PDO $conn, Poll $pollObj, Array &$session, Array &$resultsCalcArray) {
   // implementation


   //... at some point
   $session['total'] = $total;
}

 

Here's a small test script you should be sure you understand now and in the future:

<?php

/*
    Illustrating PHP Function parameters.
*/

class o {
    private $v = 0;
    
    public function inc() {
        $this->v++;
    }
    
    public function getV() {
        return $this->v;
    }
}

function testParams(o $obj) {
    for ($x=0; $x < 10; $x++) {
        $obj->inc();
    }
    return $obj->getV();
}

function testParams2($fp, $text) {
    $text = "1." . $text . PHP_EOL;
    fwrite($fp, $text);    
}

// Pass Array by Reference
function testParams3(Array &$globArray) {
    array_push($globArray, 'grape');
}

// Globally scoped variables
$globArray = ['apple', 'banana', 'peach'];
$obj = new o();
$r = fopen("/tmp/resource.txt", "w+");
$text = "This is some text.";
//

$retVal = testParams($obj);
echo $retVal . PHP_EOL;  
echo $obj->getV() . PHP_EOL;

echo PHP_EOL;
echo "text before testParams2: \n";
echo "\t$text" . PHP_EOL;
testParams2($r, $text);
rewind($r);
$fileData = fgets($r, 4096);
echo "File Data:\n";
echo "\t$fileData";
echo "text After testParams2::\n";
echo "\t$text" . PHP_EOL;

echo PHP_EOL;
echo "Array Before testParams3:\n";
var_dump($globArray);

echo PHP_EOL;
testParams3($globArray);
echo "Array After testParams3:\n";
var_dump($globArray);

 

I posted it to 3v4l so you could experiment with it if you choose.

 

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.