0
\$\begingroup\$

Display visitor's number on your web page.

The "visitors" table has only one column (visitor_count) and only one row. The column's initial value is 0.

I know goto should not be used but then the code would be nested if conditions:

if (...) {
 if (...) {
  if (...) {
  .
  .
  .
  // display successful result
  echo "You are visitor number: ";
  printf("%10d", $row["visitor_count"]);
  }
 }
}

Is there any other solution other than goto and nested if conditions such that "Rest of HTML" always show up even if php script encounters an error (like, database connection failed, table visitors not found, etc.)?

Code is below. Please do the code review.


index.php


<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <meta name="viewport" content="width=device-width, initial-scale=1">
    <title>Visitor Count</title>
</head>

<body>

<?php

$servername = "localhost";
$username = "root";
$password = "root";
$dbname = "exp";

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

// check db connection
if ($conn == false) {
    echo("Connection failed: " . mysqli_connect_error());
    goto END;
}

// execute query
$sql = "UPDATE visitors SET visitor_count = visitor_count + 1";
$result = mysqli_query($conn, $sql);

// check result
if ($result == false) {
    echo("Query failed: " . $sql. "<br>". "Error: " . mysqli_error($conn));
    goto DB_END;
}

// execute query
$sql = "SELECT visitor_count FROM visitors";
$result = mysqli_query($conn, $sql);

// check result
if ($result == false) {
    echo("Query failed: " . $sql. "<br>". "Error: " . mysqli_error($conn));
    goto DB_END;
}

// get number of rows
$num_rows_expected = 1;
$num_rows_got = mysqli_num_rows($result);

// verify number of rows got
if ($num_rows_got != $num_rows_expected) {
    echo("Error: Mismatch: Number of rows expected = " . $num_rows_expected . ", Number of rows got = " . $num_rows_got);
    goto RESULT_END;
}

// get a row
$row = mysqli_fetch_assoc($result);

// check if we got a row
if (($row == false) || ($row == null)) {
    echo("Error: Got no row (data).");
    goto RESULT_END;
}

// display successful result
echo "You are visitor number: ";
printf("%10d", $row["visitor_count"]);

RESULT_END:
    // free the memory used by $result
    mysqli_free_result($result);

DB_END:
    // close db connection
    mysqli_close($conn);

END:

?>

<!-- Rest of HTML should show up in all cases (successful cases as well as in error cases) -->
<br><br>Rest of HTML

</body>
</html>

\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

A good method to avoid goto and nesting is exception handling. Something like this:

// database details
$servername = "localhost";
$username   = "root";
$password   = "root";
$dbname     = "exp";

// activate reporting
$driver = new mysqli_driver();
$driver->report_mode = MYSQLI_REPORT_ALL;

try {
    // create connection, if it fails an exception will be thrown
    $mysqli = new mysqli($servername, $username, $password, $dbname);

    // add our visit to the total
    $mysqli->query("UPDATE visitors SET visitor_count = visitor_count + 1");

    // get total number of visits
    $result = $mysqli->query("SELECT visitor_count FROM visitors");
    $row    = $result->fetch_assoc();
    echo "You are visitor number: ";
    printf("%10d", $row["visitor_count"]);

} catch (mysqli_sql_exception $exception) {
    // log and report error, then continue
    echo "<br><br>A database error has occurred.<br><br>";
    error_log("Visitor count database error: " . $exception);
}

In the code I used the object-oriented style instead of the procedural style, but either will do.

I left out quite a bit. I don't see the point of testing if there is one row, but you can, of course, add that. I didn't free the result, or close the database connection, because that will be done automatically when the script terminates.

As soon as any database error occurs this code will jump to the catch block. This will however not work for normal PHP errors. You might, for instance, think that you would need to check that $row["visitor_count"] exists, but I don't think so. As I said before, I assume that the single row exists in the database. If that row couldn't be retrieved it must be due to a database error, which would generate an exception.

Note 1: If your website gets very busy, and you want the visitor number to be very accurate, you might want to consider putting the 2 queries in a transaction, but I wouldn't bother. In my opinion it is completely irrelevant that the visitor count might be out by a few when the site is that busy.

Note 2: Reporting exact database errors to visitors might not be a good idea. It could be a security risk. It is better to report: "A database error has occurred.", to the visitor, and send the exact error to yourself or to a log file.

\$\endgroup\$
7
  • \$\begingroup\$ "In the code I used the object-oriented style instead of the procedural style" I can't see where is the object-oriented style in your code. \$\endgroup\$
    – Zakk
    Commented Apr 10, 2022 at 17:41
  • \$\begingroup\$ @Zakk: This creates an object: $mysqli = new mysqli(...);. \$\endgroup\$ Commented Apr 10, 2022 at 17:57
  • \$\begingroup\$ @KIKO printf("You are visitor number: %10d", $mysqli->query("SELECT visitor_count FROM visitors")->fetch_assoc()["visitor_count"] ?? 1); (multiline it for readability but remove single-use variables.) \$\endgroup\$ Commented Apr 11, 2022 at 1:45
  • \$\begingroup\$ $exception->__toString() is very strange approach, because the only __toString()'s purpose is to be called in the string context (i.e. inside the concat operator) automatically. \$\endgroup\$ Commented Apr 17, 2022 at 8:00
  • \$\begingroup\$ @YourCommonSense: Yes, I agree, it looks somewhat strange, but it does exactly what is needed. It turn the exception object into a string for logging. Here is an output example. This way of logging exceptions comes from the PHP manual. To have more control over the output one would use the normal mysqli_sql_exception methods. \$\endgroup\$ Commented Apr 17, 2022 at 8:24