Shoutbox system

Question

I wrote a little Shoutbox System which is working fine, but I want to know if there is something I can improve about this code.

Here is my Shoutbox Overlay:

<div class="article_body" id="article_body" name="article_body">
<?php
    require('./tools/php/connector/connector.php');

    $query = "SELECT * FROM `shoutbox` ORDER BY entryid DESC LIMIT 5";
    $stmt = $db->prepare($query);
    $stmt->execute();
    $result = $stmt->get_result();
    $stmt->close();
    while($obj = $result->fetch_object()) {
        if ($obj != null || !empty($obj)) {

            echo("
            <p style='font-size: 14px;'>
                $obj->text
            </p>
            <table style='width: 100%; font-size: 11px;'>
                <tr>
                    <td style='text-align: start;'>
                        Von: <a style='font-size: 11px'; href='?".$obj->user."'>".$obj->user."
                    </td>
                    <td style='text-align: end;'>
                        Am: ".$obj->date_time."
                    </td>
                </tr>
            </table>
            <hr>
            ");
        }
        else {
            echo("<center>Shout-Box is empty</center>");
        }
    }
    if ($_SESSION['besucht'] == 1 && $_SESSION['user-ban-status'] - 1 == 0) {
        echo("<form action='./tools/php/shoutbox/shoutbox.php' method='POST'>
            <input type='text' name='shout' maxlength='500' style='width: 186px;'><button>Senden</button>
        </fomr");
    }
    if ($_SESSION['besucht'] == 1 && $_SESSION['user-ban-status'] - 1 == 1) {
        echo("<h5 style='color: red;'>You Are Banned. This Function Isn't Allowed For Banned Members.</h5>");
    }
    mysqli_close($db);
?>
</div>

The $_SESSION['besucht'] is for the Visited and logged in check. The $_SESSION['user-ban-status'] is to check if the user is banned and if that is true the post function is disabled. I limited the View per to the latest 5 entrys in my database so only the newest thing would be shown.

Here is my Backend Script for the input of the Shoutbox:

<?php
    if (!empty($_POST['shout']) || $_POST['shout'] != null) {

        session_start();

        if (!empty($_SESSION['user-username']) || $_SESSION['user-username'] != null) {
            require('../connector/connector.php');

            $text = $_POST['shout'];

            $user = $_SESSION['user-username'];

            date_default_timezone_set("Europe/Berlin");
            $timestamp = time();
            $datum = date("Y-m-d H:i:s",$timestamp);

            echo($text."<br>");
            echo($user."<br>");
            echo($datum."<br>");
            $stmt = $db->prepare("INSERT INTO `shoutbox`(`text`, `user`, `date_time`) VALUES (?,?,?)");
            $stmt->bind_param('sss', $text, $user, $datum);
            $stmt->execute();

            mysqli_close($db);
            header("Location: ../");
            exit;
        }
        else {
            header("Location: ../");
            exit;
        }
    }
    else {
        header("Location: ../");
        exit;
    }
?>

Hey, welcome to Code Review! Here we review working code and try to make it better. What we cannot do is solve your problem for you or make your code do new things. Have a look at our help center for more information. — Graipher
– Graipher, Commented Aug 2, 2019 at 12:16
My code works. I'm just asking for a suggestion for a hint or a recommendation for this code. If you do something like that you can also ask if someone has an idea for their own idea xD I don't think that will be a shame or against the "rules" — NvrKill
– NvrKill, Commented Aug 2, 2019 at 12:20
From your question it sounds like what you are asking is for us to implement a new feature: "So I mean how can I add numbers for Pages for the useres to swipe forward and backward to the discussions in the Shoutbox?" If your code cannot currently do this, it is off-topic to ask about here. What you can ask about instead is, describe what your code does do and how that could be done in a better way. — Graipher
– Graipher, Commented Aug 2, 2019 at 12:22
I believe this question has been edited to be on-topic, so I have voted to reopen. — mickmackusa
– mickmackusa, Commented Aug 3, 2019 at 21:01

mickmackusa · Accepted Answer · 2019-08-04 22:50:27Z

I recommend writing session_start(); first and unconditionally.
!empty() performs two checks: if the variable isset() AND contains a non-falsey value. This means $obj != null is not necessary. That said, your while() loop will halt if $obj is falsey.
There's nothing wrong with using prepared statements, but for the record your first query will be just as stable/secure without it.
I noticed mysqli_close($db);. You should keep all of your query syntax object-oriented.
Rather than declaring new variables to feed to $stmt->bind_param() (single-use variables), just write the original variables as parameters. There isn't much benefit in adding more variables to global scope.
It looks like all roads lead to:
```
header("Location: ../");
```
If this is true for you actual project script, write a single if block, then whether its contents are executed or not, after the condition block execute your redirect.

Dharman · Accepted Answer · 2021-10-19 14:56:52Z

Don't mix database logic and display logic (HTML). Prepare the data first and then display it in HTML when you have everything ready.
Try to avoid mysqli in new projects. Use PDO whenever possible. If you must use mysqli then stick to the object-oriented style.
Don't close connection object or the statement. PHP will do that for you and you avoid mistakes by not doing it manually.
Don't use date() and time() functions. Use DateTime class.
Don't put parentheses after echo. It's not a function and these parentheses are extremely confusing. Same applies to require
Reduce cyclomatic complexity by performing inverse checks. exit will kill the script so you can use that to finish early without wrapping whole code blocks in braces.
Use strict comparison (===).
Use htmlspecialchars() to avoid XSS.

Stack Exchange Network

Shoutbox system

2 Answers 2

You must log in to answer this question.

Linked

Hot Network Questions

Shoutbox system

2 Answers 2

You must log in to answer this question.

Linked

Related

Hot Network Questions