PHP5.4 to PHP7.2 migration help

ibraheembcm · November 2, 2018, 9:47am

I’ve been struggling with some migration work and I was wondering if someone might be able to shed some light on why my code isn’t working.

My site connects to a MySQL db and pulls data into an array and then displays it in another file elsewhere on the site. My code to connect to the database is:
// Connect to Database

if(!session_id ()){
      session_start();
}

function dbConnect() {
	/* conecting to the mySQL database */
	$servername = "localhost";
	$username = "xxx";
	$password = "xxx";
	$dbname = "xxx";

	// Create connection
	$db = mysqli_connect($servername, $username, $password, $dbname);

	// Check connection
	if (!$db) {
    	die("Connection failed: " . mysqli_connect_error());
	}
	echo "Connected successfully";
	return $db;
}

// Disconnect from Database

function dbClose($dbConnection) {
	 mysqli_close($dbConnection); // Close connection to database.
}

// Get Prayer times
$prayerTimes = getPrayerTimes();
function getPrayerTimes() {
	// Return the details for this month
	$db = dbConnect();// Connect to database
	
	if($conn !== false ) {
		// Fetch current date
		$thisday = date("Ymd");

		$result = $db->query("SELECT GregDate, Fajr, FajrJam, Sunrise, Dhuhr, DhuhrJam, Asr, AsrJam, Maghrib, Ishaa, IshaaJam FROM PrayerTimes WHERE GregDate = '$thisday'");
		while($row=$result->fetch_assoc()){
			$prayerTimes[] = $row;
		}

		return $prayerTimes;
		
		echo mysqli_error($db);
		dbClose($db);
		
	} else {
		die("Connection failed: " . mysqli_connect_error());
	}
}

My page code is:
// Return the details for this month
getPrayerTimes();
echo $prayerTimes[‘FajrJam’];

I do not get an output from this. I have a strong suspicion that this is because the output of GregDate is a string and so I’m not sure if I can compare it with $thisDay (although this used to work in PHP5 and the spec says the output of the date function is also a string). When I var_dump the output is NULL. The GregDate entries in the db table are in the format of Ymd i.e. 2019-01-01.

Any help would be much appreciated.

phdr · November 2, 2018, 2:21pm

For the posted code, you will get a returned NULL value if the query failed due to an error OR the query didn’t match any data.

For the possibility of the query failing, you ALWAYS need error handling for any statements that can fail. You have error handling for the connection, but not for the query (and trying to echo the mysqli_error() information after you have returned from the function is pointless since the return stops code execution.)

The easiest way of adding error handling, without needing to add logic for every statement that can fail (Keep It Simple - KISS), is to use exceptions and in most cases let php catch the exception, where it will use its error_reporting, display_errors, and log_errors settings to control what happens with the actual error information. With exceptions, your code only deals with error free execution, since an error will throw an exception and execution will transfer to the exception handling.

To enable exceptions for mysqli errors, add the following line of code before the point where you make the database connection -

mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);

Next, set php’s error_reporting to E_ALL and display_errors to ON, in the php.ini on your system so that php will help you by reporting and displaying all the errors it detects. Stop and start your web server to get any changes to the php.ini to take affect. These are apparently not set since you have an undefined php variable ($conn) that should be producing an error. This, combined with the above line of code will get any connection/query errors to be handled by php’s exception handling.

Then, remove the error handling logic you have now for the connection in both the dbConnect() function and repeated in the getPrayerTimes() function (Don’t Repeat Yourself - DRY.)

For the data matching or not, if the GregDate column is defined as a DATE data type, the query should find data if there are row(s) that match the current date.

Lastly, this code contains other bad practices and problems that are either using unnecessary lines of code or are using extra resources. Some points -

Unconditionally start the session. Your main code should unconditionally execute one session_start() statement. There is no point in having any conditional logic around the session_start().
Don’t create a database connection inside the getPrayerTimes() function just to execute one query. This will consume a separate database connection for each function you do this for inside an instance of your script. Your main code should create ONE database connection, then supply it as a call-time parameter to any function that needs it.
Before you fetch the data from the SELECT query, you should initialize the array variable to an empty array so that if the query doesn’t match any data you will get an empty array back from the function call. Your main code should then test the returned value before using it and display an appropriate message if there is no data to display.
Php automatically closes database connections when the script ends, so there is no need for you to do so.
User defined functions should either be in an external .php file and required when needed or at a minimum, grouped together near the start of your code. One point of user defined functions is to make the main code easier to read. By leaving the function definitions in-line and cluttering up the main code, you are not achieving this goal.
Your My page code is: example isn’t actually using the result from the function call, which will result in a NULL result. However, your posted code is calling the function and assigning the returned value to a main program variable - $prayerTimes = getPrayerTimes(); Which case is it? (This is a good example of why the function definitions should not be in-line and cluttering up your main code, so that you can see what the main code is actually doing.)

ibraheembcm · November 2, 2018, 2:51pm

Thank you very much for such a detailed reply. It is clear that there is much I still need to learn!
Please bear with me while I try to digest everything you have kindly pointed out.

benanamen · November 2, 2018, 4:59pm

You are trying to compare 20181102 to 2018-11-02.

$thisday = date("Ymd");

Should be

$thisday = date("Y-m-d");

ibraheembcm · November 2, 2018, 5:06pm

Thank you, I did catch and correct it. So far I’ve re-written following @phdr’s advice:

// Connect to Database 

session_start();
function dbConnect() {
	mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);
	static $conn;
    if ($conn===NULL){ 
        $conn = mysqli_connect("localhost", "brightch_wise", "t1+W9g8]nlwX", "brightch_wise");
    }
    return $conn;
}

// Gat Prayer Times 

function getPrayerTimes() {
	$conn = dbConnect();
	
	// Fetch current date
	$thisday = date("Y-m-d");

	$result = $conn->query("SELECT GregDate, Fajr, FajrJam, Sunrise, Dhuhr, DhuhrJam, Asr, AsrJam, Maghrib, Ishaa, IshaaJam FROM PrayerTimes WHERE GregDate = '$thisday'");
	echo $result;
    //$prayerTimes = mysqli_fetch_assoc($result);
    //return $prayerTimes;			
}

The result is not returning an object, which I’m just working through at the moment.

benanamen · November 2, 2018, 5:09pm

I would highly recommend you use PDO instead of Mysqli. Here is a good tutorial to get you going.

ibraheembcm · November 3, 2018, 10:24am

Is it really that much better for single database use?
I have looked over it a few times but never thought I needed it for this reason.

It also does seem a bit more complicated than the traditional methods.

benanamen · November 3, 2018, 3:56pm

Nearly all applications use a single database. PDO is actually easier to use.