PHP OOP coding conventions

Question

I'm brand new at OOP PHP and i would like to get as many of the coding conventions right as fast as possible. I made this tiny guestbook script, and i would like to know if there's anything not done as it should.

Index.php:

<?php
$username = "root";
$password = "";
$database = "oop";
$host = "localhost";

mysql_connect($host, $username, $password);
mysql_select_db($database);
?>
<html>
<head>
</head>
<body>
    <?php
    include("views/gaestebog.php");
    ?>
</body>
</html>

Guestbook.php, the class:

<?php
class Gaestebog {

    public function __contruct() {

    }

    public function getPosts() {
        $query = mysql_query("SELECT * FROM gaestebog");
        while ($row = mysql_fetch_array($query)) {
            echo '
            <tr>
                <td>'.$row['navn'].'</td>
            </tr>
            <tr>
                <td>'.$row['besked'].'</td>
            </tr>
            ';
        }
    }

    public function addPost($navn, $besked) {
        mysql_query("INSERT INTO gaestebog VALUES('', '$navn', '$besked')");
    }
}
?>

and guestbook.php, the view:

<?php
include("classes/Gaestebog.php");
$gaestebog = new Gaestebog();
if (isset($_POST['opret'])) {
    $navn = $_POST['navn'];
    $besked = $_POST['besked'];
    $gaestebog->addPost($navn, $besked);
}
?>
<table>
    <?php
    $gaestebog->getPosts();
    ?>
</table>

<hr />

<form action="" method="post">
    <table>
        <tr>
            <td>Navn:</td>
            <td><input type="text" name="navn" value="Patrick" /></td>
        </tr>
        <tr>
            <td>Besked:</td>
            <td><input type="text" name="besked" value="Hej med dig !!" /></td>
        </tr>
        <tr>
            <td><input type="submit" name="opret" value="Opret" /></td>
        </tr>
    </table>
</form>

Seems better suited for http://codereview.stackexchange.com. — deceze, Nov 09 '12 at 08:25
You might want to take a look at the PHP manual online about your use of database operations for MySQL for example `mysql_query()` is no longer suggested http://php.net/manual/en/function.mysql-query.php one of the alternatives is to use the `mysqli` interface with is OOP. — JRSofty, Nov 09 '12 at 08:27
instead of using `mysql` go for `mysqli` or `PDO`. Use parameterized query. make a use of database abstraction class instead of using core database functions in your classes. Thats all I can think of now — WatsMyName, Nov 09 '12 at 08:29
When i use mysqli, should i be creating a new object inside every class of the mysqli connection? — Patrick Reck, Nov 09 '12 at 08:43
@PatrickReck: No, you create one instance, and pass it around you objects/methods. — Madara's Ghost, Nov 09 '12 at 09:28

hakre · Answer 1 · 2012-11-09T08:37:44.327

This is often called procedural programming with outdated libraries:

mysql_connect($host, $username, $password);
mysql_select_db($database);

And that was only the beginning of your script. Maybe it's better you don't focus so much on fancy programming terms but instead just get the work done.

Example guestbook.php:

<?php
require('bootstrap.php');

$page = new Htmlpage('My Guestbook');
$page->start($_SERVER['REQUEST_URI']);

$gaestebog = new Gaestebog();

echo '<table border="1">';

foreach ($gaestebog->getPosts() as $post)
{
    $row = new Htmlencoded($post);

    echo <<<OUT
            <tr>
                <td>$row['navn']</td>
            </tr>
            <tr>
                <td>$row['besked']</td>
            </tr>
OUT;
}
echo '</table>';

$page->closeRequest();
?>

Otherwise if you want to learn about object oriented programming, use object interfaces and practice, practice, practice.

Start with something simple, like a form field. Not with a whole script.

What the hell is "Gaestebog"? – samayo Nov 12 '13 at 17:25 — samayo, Nov 12 '13 at 17:25
@Simon_eQ: Did you mean: *Wat de hel is "Gaestebog"?* ? :D – hakre Nov 13 '13 at 08:34 — hakre, Nov 13 '13 at 08:34

score 4 · Answer 2 · edited May 25 '19 at 11:54

4

Yes, the first, and most obvious thing to note is your vulnerability to SQL injection.

Please, don't use mysql_* functions in new code. They are no longer maintained and the deprecation process has begun on it. See the red box? Learn about prepared statements instead, and use PDO or MySQLi - this article will help you decide which. If you choose PDO, here is a good tutorial.

Also, it's considered good practice to separate logic from presentation (i.e. PHP from HTML).

edited May 25 '19 at 11:54

Zoe

27,060
21
118
148

answered Nov 09 '12 at 08:28

Madara's Ghost

172,118
50
264
308

How would i separate it even more than i've done here? – Patrick Reck Nov 09 '12 at 08:46
@PatrickReck: Well, you need very little PHP code on your **view** or **template** (that's what it's called). Form processing, heavy business logic, database interactions, those should be on their own files, with zero HTML output on them. – Madara's Ghost Nov 09 '12 at 09:27

score 4 · Answer 3 · answered Nov 09 '12 at 08:31

Couple of points:

Don't use the deprecated mysql library, try PDO or MySQLi instead
__contruct() has a typo, should be __construct().
Don't return raw HTML from get methods such as getPosts(), instead return an array of posts and let the view create HTML if that's the output you want
Protect against, SQL Injection, your INSERT query is vulnerable

PHP OOP coding conventions

3 Answers3