0
\$\begingroup\$

New to PHP and was just wondering if any improvements can be made, maybe increase efficiency or just clean up code?

I know I'm using an old MYSQL maybe we can update it. Just want to make sure the connection can be as best to its ability. Also, if you can, can you explain on how to improve security?

The JSON gets picked up by Swift iOS app.

<?php

$dbhost = "website";
$dbuser = "user";
$dbpass = "password";
$dbname = "database";

$connection = mysql_connect($dbhost,$dbuser,$dbpass);

// Checking if connection can be established
if(!$connection){

    die("Connection Failed");

} else {

// Selecting Database
$dbconnect = mysql_select_db($dbname, $connection);

// Checking if it can connect to Database
if(!$dbconnect){

    die("Unable to connect to Database");

} else {

    $query = "SELECT * FROM blogs";
    $resultset = mysql_query($query, $connection);
    $records= array();

    // Loop and check all records and add to array
    while($r = mysql_fetch_assoc($resultset)) {

        $records[] = $r;
    }

    // Output data as JSON
    echo json_encode($records);
 }
}

?>
\$\endgroup\$
4
  • 1
    \$\begingroup\$ Please, don't use mysql_* functions for new code. They are no longer maintained and the community has begun the deprecation process, and mysql_* functions have been officially removed in PHP 7. Instead you should learn about prepared statements and use either PDO or mysqli_*. If you can't decide, this article will help to choose your best option. \$\endgroup\$ Commented Jul 6, 2017 at 15:26
  • 1
    \$\begingroup\$ Please describe what the code does in the title. Everyone here want to "make code more efficient and to clean it up". \$\endgroup\$ Commented Jul 6, 2017 at 15:28
  • 1
    \$\begingroup\$ You have an else statement after a die() statement, which is not needed because if the IF statement hits the die() the page will stop loading, so you can eliminate 2 lines of code with that. You can do the same with the next if-else statement. Also, you are not actually displaying any errors if your connection fails, it just says Connection Failed instead of something useful for troubleshooting. \$\endgroup\$ Commented Jul 6, 2017 at 15:33
  • \$\begingroup\$ I updated my answer with a few refactors. \$\endgroup\$ Commented Jul 6, 2017 at 15:54

1 Answer 1

1
\$\begingroup\$

Consider this:

<?php
    $dbhost     = "website";
    $dbuser     = "user";
    $dbpass     = "password";
    $dbname     = "database";
    $connection = mysql_connect($dbhost, $dbuser, $dbpass);
    //Kill page if connection could not be established
    //I also added mysql_error() for it to display an error if there was an issue, so you can troubleshoot.
    if(!$connection) { 
        die("Connection Failed:" . mysql_error()); 
    }

    $dbconnect = mysql_select_db($dbname, $connection);
    //Same as before, kill page if there was an issue, and display an error.
    //No need for the IF-ELSE statements, if the `die()` function is hit, the page stops loading anyways.
    if(!$dbconnect) { 
        die("Unable to connect to Database: " . mysql_error()); 
    }

    $query     = "SELECT * FROM blogs";
    $resultset = mysql_query($query, $connection);
    $records   = array();
    // Loop and check all records and add to array
    while($r = mysql_fetch_assoc($resultset)) {
        $records[] = $r;
    }
    // Output data as JSON
    echo json_encode($records);
?>

But I highly recommend switching to mysqli_* or PDO statements:

Please, don't use mysql_* functions for new code. They are no longer maintained and the community has begun the deprecation process, and mysql_* functions have been officially removed in PHP 7. Instead you should learn about prepared statements and use either PDO or mysqli_*. If you can't decide, this article will help to choose your best option.


Here is a refactor to mysqli_*

<?php
    $dbhost     = "website";
    $dbuser     = "user";
    $dbpass     = "password";
    $dbname     = "database";

    $records   = array();

    $connection = new mysqli($dbhost, $dbuser, $dbpass, $dbname);
    if($connection->connect_errno > 0){
        die('Unable to connect to database [' . $connection->connect_error . ']');
    }
    $query     = "SELECT * FROM blogs";
    if(!$result = $connection->query($query)){
        die('There was an error running the query [' . $connection->error . ']');
    }
    while($row = $result->fetch_assoc()){
        $records[] = $row;
    }
    echo json_encode($records);
?>

There are a couple of differences here, the first connection checks for an actual connection to the database AND to the table in 1 statement.

Another difference is it shows an error specifically for the query if there was a problem, and not for a table connection error in the 2nd error check.


Here is a PDO refactor:

NOTE: I don't actively develop with PDO, I may have missed something here.

<?php
    $dbhost     = "website";
    $dbuser     = "user";
    $dbpass     = "password";
    $dbname     = "database";
    $charset = 'utf8';

    $records   = array();

    $dsn = "mysql:host=$dbhost;dbname=$dbname;charset=$charset";
    $opt = [
        PDO::ATTR_ERRMODE            => PDO::ERRMODE_EXCEPTION,
        PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC,
        PDO::ATTR_EMULATE_PREPARES   => false,
    ];
    $pdo = new PDO($dsn, $dbuser, $dbpass, $opt);

    $query     = "SELECT * FROM blogs";
    $stmt = $pdo->query($query);
    while ($row = $stmt->fetch()) {
        $records[] = $row;
    }
?>
\$\endgroup\$
12
  • \$\begingroup\$ @BroSimple I generally find mysqli_* much easier to write than the old mysql_* statements. \$\endgroup\$ Commented Jul 6, 2017 at 15:58
  • \$\begingroup\$ It's a lot less code but performance is the same to mysql_* ? \$\endgroup\$
    – WokerHead
    Commented Jul 6, 2017 at 15:59
  • 1
    \$\begingroup\$ @BroSimple No, performance is better with mysqli_*, especially when using Prepared statements, as it is more optimized. \$\endgroup\$ Commented Jul 6, 2017 at 16:01
  • 1
    \$\begingroup\$ @BroSimple Also, it's not really less code. I just took out all the comments. The difference in code is about 15 characters. Each are 11 lines if you take out all of the comments, and between 628 and 643 characters. So it is a tiny bit shorter to use mysqli_*. PDO is a few characters shorter but more lines. \$\endgroup\$ Commented Jul 6, 2017 at 16:05
  • \$\begingroup\$ I will be learning more about mysqli thanks again! \$\endgroup\$
    – WokerHead
    Commented Jul 6, 2017 at 16:23

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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