Version 2: Display visitor's number on your web page (changed code after getting answer for first version)

Question

Version 2: Display visitor's number on your web page (changed code after getting answer for first version).

The first version is here: 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.

The new code is below:

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";

$internal_error_code = 0;

try {

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

    // check db connection
    if ($conn == false) {
        throw new Exception("Connection failed: " . mysqli_connect_error());
    }

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

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

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

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

    $internal_error_code = 4;
    // 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) {
        throw new Exception("Error: Mismatch: Number of rows expected = " . $num_rows_expected . ", Number of rows got = " . $num_rows_got);
    }

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

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

    // display successful result
    echo ("You are visitor number: " . $row["visitor_count"]);

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

    // close db connection
    mysqli_close($conn);

} catch (Exception $e) {

    if ($internal_error_code >= 4) {
        // free the memory used by $result
        mysqli_free_result($result);
    }
    if ($internal_error_code > 1) {
        // close db connection
        mysqli_close($conn);
    }
    echo ("Error: Some error occurred. Internal error code = " . $internal_error_code);

} // end of try-catch block

?>

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

</body>
</html>

I'm a bit disappointed that you took away so little from my answer on version 1. That $internal_error_code hardly serves an useful purpose, you don't use mysqli exceptions at all (why?), and nothing is done with the messages in the exceptions. In other words, you don't use $e->getMessage() in the catch block. I guess the latter is for security purposes, like I advised, but you still need to do something with the error message itself. You are using exceptions, but in a quite different way. — KIKO Software, Commented Apr 11, 2022 at 19:22
@Amit ask yourself if your end users actually care about the cause of a potential error. [pause] No, they do not. They will probably spend less than half a second acknowledging the visitor count before looking at the rest of the page. If query breaks for any reasons (and it shouldn't in production), then you could either not show anything, or ?, or omit the text entirely. ~90% of your script can be safely deleted. You might just execute the two queries and move on. No conditions and no connection closing. — mickmackusa, Commented Apr 11, 2022 at 21:58
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 fille — Your Common Sense, Commented Apr 17, 2022 at 10:01
I'm going to issue a broadband warning to all of the parties involved here: the comments on the question and answers got out of hand. Ordinarily I'd move them to chat, but enough of them were unfriendly that I don't consider preserving them a good idea. Amit, reviewers can be critical. It's nothing personal. Please don't respond in an attacking manner if you feel attacked by a comment. If a moderator needs to get involved, use flags instead. — Mast, Commented Apr 20, 2022 at 6:25
While people can be harsh, ignoring advice previously given can bring that out of people. Some would say it's offensive to post new questions without following the advice provided in older ones. It is usually preferred to wait a couple of days between posts about the same function either way. — Mast, Commented Apr 20, 2022 at 6:25

Your Common Sense · Accepted Answer · 2022-04-17 08:58:13Z

I won't post any code here because you apparently not in the mood of using any good practice shown to you.

Instead, I'd try to review your blunders verbatim, so even if you ignore it, some other reader may find it useful, as your mistakes are very common.

The sin of overengineering

We all, when learning, tend to do even simplest things unnecessary overcomplicated. Simply because we don't know the simpler ways. A trivial database operation, that, in the hands of a skilled programmer will take a few rows, you managed to stretch between several screens of code.

Always review your code in order to see whether it can be optimized. If you don't have enough experience to do so, you can post your question on the codereview site to get some suggestions. For example, you can be told on that site, that instead of checking the outcome for the every database operation and throwing exceptions manually, you can configure PHP to throw exceptions automatically.

Error handling

Is another weak spot for the newbie programmers. Although they somehow understand that someday their site will have many visitors, but in reality, a newbie programmer always programs as though they always be the sole user of the site who is sitting at its console. Which, of course, is wrong.

This is why there are two error reporting modes: development and production. Although being familiar with development mode where all errors are bluntly printed on the screen, newbie programmers have quite a vague idea on the second one. While it's not a rocket science either: all they need is to stop displaying errors on the screen, and start logging them. An error log is a marvelous invention. It's sort of a time machine: it tells you what happened to your site a time ago. Very convenient. All you need is to peek in to the logs either in case something is wrong, or just in case, to see if everything went smoothly during your absence.

Having the error log, you won't have to bother your visitors with error messages at all, or ask them to be the messenger who conveys the error message to you.

Logic is not a joke

Unlike many human trades, programming heavily relies on the logic. It is important for the programmer to understand the basic logical evaluations. And a program written must be logical as well. For example, if you aren't going to use the error message anyhow, it makes very little sense to laboriously collecting the error details.

Arrogance is one of the seven deadly sins

When asking for (and being given) the advise, it's a good idea to follow it. Or, if you don't agree with some part of it or not fully understood it, it would be wise to ask for the clarifications. People are always willing to help, all you need is to ask, to provide some feedback. Without asking for explanations, and without following the advise provided, it makes little sense to ask for the advise at all.

Then you are not ready for them yet. There is a thing called learning curve. At first we all do not understand advanced topics - so that's why we have to start from basic ones. Learn harder and some day you will start to understand general statements that can be found. — Your Common Sense, Commented Apr 17, 2022 at 10:09
@Amit The above answer did put into words, far more eloquently than I ever could, exactly what I thought when I wrote my comment on your version 2 question. — KIKO Software, Commented Apr 17, 2022 at 20:09

KIKO Software · Accepted Answer · 2022-04-18 10:16:43Z

I'll try to put into my own words why I am disappointed that you seem to misunderstand my suggestions in version 1.

In your original question you asked: "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", and I have tried to answer that question.

I got rid of all the separate error checks by introducing mysqli_sql_exception. A simply way to tackle all possible database errors.

In your version 2 you don't use MySQL exceptions and all the individual error checks reappeared. You've somewhat ignored my answer. Why do I say that? Let's look at what happens in your code:

Hand cranked exceptions

Instead of using MySQL exceptions, you generate your own exceptions. That's more code, and you loose some information in the process. An example is the first exception you throw:

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

// check db connection
if ($conn == false) {
    throw new Exception("Connection failed: " . mysqli_connect_error());
}

If the connection fails you only get the message:

Error: Some error occurred. Internal error code = 1

This only tells you that the connection failed, but not why, because:

Ignoring your own exceptions

You completely ignore your own exceptions. In the catch block you do not refer to the exception that occurred at all, you only output an error number. Now I know you did this on my advice, and it is easily corrected, but still, it is very strange.

Using lots of code to create information that's already there

For this we have to look at an example message that would have been logged if you had used MySQL exceptions and logged them. When mysqli_connect() fails you can get:

mysqli_sql_exception: Access denied for user 'root'@'localhost' (using password: YES) in /test.php:15 Stack trace: #0 /test.php(15): mysqli->__construct() #1 {main}

There's a lot to unpack here. It says: "Access denied", which means the server is running but the credentials used are invalid. It also says: "in /test.php:15" which tells you which script is involved (test.php) and on which line number (15) the error occurred.

Your $internal_error_code crudely replaces the line number, and you throw away any other useful information. $internal_error_code is completely superfluous, you can simply use the line number of the exception: Exception::getLine().

But what about the errors MySQL exceptions do not report?

Yes, there are some errors in your code that MySQL exceptions do not catch, for instance:

$internal_error_code = 4;
// 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) {
    throw new Exception("Error: Mismatch: Number of rows expected = " . $num_rows_expected . ", Number of rows got = " . $num_rows_got);
}

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

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

Note how these two errors overlap. If the $result set contains 1 row, it is unlikely you cannot fetch it. In my original answer I said: "I don't see the point of testing if there is one row, but you can, of course, add that." So, how would I have added that error to my answer? Something like this:

// get total number of visits
if (mysqli_num_rows($result) != 1) {
    error_log("Visitor count database error: The visitors table does not contain 1 row of data.");
}
$row = $result->fetch_assoc();

The problem would be logged, if it occurred, with this simple check. Note that all the other exceptions you programmed would have been handled if you use MySQL exceptions.

What else did you ignore?

You kept the mysqli_free_result($result); and mysqli_close($conn); in your code, even though they are not needed. That's fine, but you use them in your catch block, based on $internal_error_code. That is risky behavior because it is error-prone.

Suppose you change your code at a later date, and you introduce another exception? You diligently changed your error numbering, but you forget to change the conditions in your catch block. Now it is possible that mysqli_free_result($result); will be called when there is no result.

I'm basically saying that more interdependent code leads to more errors in the code.

Am I my code?

We programmers tend to see our code as our little babies. We worked hard on them and it is the best we can do. Criticism, especially when it seems unjustified, it hard to deal with. Believe me, I know.

But this is the wrong attitude. Code can always be improved, and there are always things you don't know or haven't realized. It's a lesson we all need to learn the hard way.

People here are really trying to help you, the best they can, listen to them. Try to understand their reasoning. Don't kill the messengers.

Comments are not for extended discussion; this conversation has been moved to chat. — Mast, Commented Apr 22, 2022 at 12:06

Stack Exchange Network

Version 2: Display visitor's number on your web page (changed code after getting answer for first version)

index.php

2 Answers 2

The sin of overengineering

Error handling

Logic is not a joke

Arrogance is one of the seven deadly sins

Hand cranked exceptions

Ignoring your own exceptions

Using lots of code to create information that's already there

But what about the errors MySQL exceptions do not report?

What else did you ignore?

Am I my code?

Linked

Hot Network Questions

Version 2: Display visitor's number on your web page (changed code after getting answer for first version)

index.php

2 Answers 2

The sin of overengineering

Error handling

Logic is not a joke

Arrogance is one of the seven deadly sins

Hand cranked exceptions

Ignoring your own exceptions

Using lots of code to create information that's already there

But what about the errors MySQL exceptions do not report?

What else did you ignore?

Am I my code?

Linked

Related

Hot Network Questions