PHP OOP - Login & DB adding/removing script

Question

I've created something that I called System, where you can login, create the files and delete the files.

It's not any kind of big project, but it's the biggest project I made in PHP OOP so far. It uses MySQL PDO database connection. Passwords are not hashed since there's no registration page. The thing I'm not sure about is that I know what is and how to use abstract classes, final classes etc. but I don't see opportunity or need to use it in the current script. I can barely imagine a situation where something like abstract class would be useful, so if you can, tell me where could I use some of this more advanced object-oriented stuff so it'll be useful.

All the files: https://github.com/quono/oopsystem

There's also an .sql file with ready-to-import database with a few examples.

It asks me to add some code here, so I put here all 3 main pages, includes are available on github.

index.php

<?php

session_start();

require 'includes/InputValidator.class.php';
require 'includes/Client.class.php';
require 'includes/User.class.php';
require 'includes/db.php';

$client = new Client();
if ($client->isLogged()) $client->redirect('system.php');

if ($_SERVER['REQUEST_METHOD'] === 'POST') {

    $nick = htmlspecialchars($_POST['nick']);
    $pass = $_POST['pass'];

    $inputValidator = new InputValidator($nick);
    if ($inputValidator->length(3, 20)) {
        $inputValidator = new InputValidator($pass);
        if ($inputValidator->length(8)) {
            $user = new User($pdo);
            if ($user->login($nick, $pass)) {
                $client->setAsLogged($user->getId($nick), $nick);
                $client->redirect('system.php');
            }   
        }
    }
    if (!isset($err)) $err = 'Nickname or password incorrect';
}
?>


<?php require 'layout/header.html' ?>


<h1>Sign in</h1>

<form method="post" action="<?=$_SERVER['PHP_SELF']?>">

    <p>Login: <input type="text" name="nick"></p>
    <p>Password: <input type="password" name="pass"></p>
    <input type="submit" value="Submit">

</form>


<?php if (isset($err)) echo '<p>' . $err . '</p>'; 

require 'layout/footer.html';

?>

system.php

<?php

session_start();

require 'includes/Client.class.php';
require 'includes/User.class.php';
require 'includes/Files.class.php';

$client = new Client();
if (!$client->isLogged()) if ($client->isLogged()) $client->redirect('index.php');

if (isset($_GET['logout'])) {
    $client->logout();
    $client->redirect('index.php');
}

require 'includes/db.php';
$files = new Files($pdo, $_SESSION['userid']);

if (!isset($_SESSION['filesData'])) $_SESSION['filesData'] = $files->fetch();

$htmlFileContent = '';

if (isset($_GET['open'])) {

    if (in_array($_GET['open'], array_column($_SESSION['filesData'], 'id'))) {  
        $file = $_SESSION['filesData'][array_search($_GET['open'], array_column($_SESSION['filesData'], 'id'))];
        $htmlFileContent .= '<hr>';
        $htmlFileContent .= '<p><b>' . $file['name'] . '.txt</b></p>';
        $htmlFileContent .= nl2br($file['text']);
    } else {
        $htmlFileContent = 'File doesn\'t exist on your system';
    }
}

if ($_SERVER['REQUEST_METHOD'] === 'POST' && isset($_POST['delId'])) {
    if (in_array($_POST['delId'], array_column($_SESSION['filesData'], 'id'))) {
        $files->remove($_POST['delId']);
        $_SESSION['filesData'] = null;
        $_SESSION['message'] = 'File has been removed successfully';
        $client->redirect($_SERVER['PHP_SELF']);
    }
}


$htmlFiles = '';

foreach($_SESSION['filesData'] as $file) {
    $htmlFiles .= '<form action method="post">';
    $htmlFiles .= '<p><span style="margin-right:15px;"><input type="submit" name="delete" value="Remove"></span>';
    $htmlFiles .= '<a href="?' . 'open=' . $file['id'] . '">' . $file['name'] . '.txt</a></p>';
    $htmlFiles .= '<input type="hidden" name="delId" value="' . $file['id'] . '">';
    $htmlFiles .= '</form>';
}

if (empty($htmlFiles)) $htmlFiles = '<i>You don\'t have any files yet</i>';


?>

<?php require 'layout/header.html' ?>

<h1>System - manage your files</h1>

<?php 

echo 'Logged as: ' . $_SESSION['nick'];
if (isset($_SESSION['message'])) {
    echo '<p>' . $_SESSION['message'] . '</p>';
    $_SESSION['message'] = null;
}

?>

<p>
    <a href="add.php">Add new file</a>
</p>
My files:
<br>
<?=$htmlFiles?>

<p>
    <a href="?logout">Log out</a>
</p>

<?=$htmlFileContent?>

<?php require 'layout/footer.html'; ?>

add.php

<?php

session_start();

require 'includes/InputValidator.class.php';
require 'includes/Files.class.php';
require 'includes/Client.class.php';
require 'includes/User.class.php';

$client = new Client();
if (!$client->isLogged()) $client->redirect('index.php');

if ($_SERVER['REQUEST_METHOD'] === 'POST') {

    $name = htmlspecialchars($_POST['name']);
    $content = htmlspecialchars($_POST['content']);

    $inputValidator = new InputValidator($name);
    if ($inputValidator->length(1, 50)) {
        $inputValidator = new InputValidator($content);
        if ($inputValidator->length(0, 10000)) {
            require 'includes/db.php';
            $files = new Files($pdo, $_SESSION['userid']);
            $files->add($name, $content);

            if (!isset($err)) {
                $_SESSION['message'] = 'File has been added successfully';
                $_SESSION['filesData'] = null;
                $client->redirect('system.php');
            }

        } else {
            $err = 'Your file size is too big';
        }

    } else {
        $err = 'File name length is not correct';
    }
}

?>

<?php require 'layout/header.html'; ?>

<h1>Add new file</h1>
<form action="<?=$_SERVER['PHP_SELF']?>" method="post">
    <p>Name:<br><input type="text" name="name" maxlength="50"></p>
    <p>Content:<br><textarea cols="60" rows="15" name="content"></textarea></p>
    <p><input type="submit" value="Save"></p>
    <a href="system.php">Cancel</a>
</form>

<?php if (isset($err)) echo '<p>' . $err . '</p>'; ?>

<?php require 'layout/footer.html'; ?>
```

Answer 1

Well, let me tell you the truth: this is not OOP at all. This is a very common approach called "ole good procedural disguised as OOP". Your code is essentially procedural. Class methods here are used as functions. You can rewrite them as functions and your code will be the same.

And even such a makeshift OOP is inconsistent.

• for example, new Files($pdo, $_SESSION['userid']); - why instead of using $client a direct access to the session variable?

• or, I would make the whole authentication into a method of the Client class, and call this method, like

  if ($_SERVER['REQUEST_METHOD'] === 'POST') {
      $client = new Client();
      $client->authenticate($pdo, $_POST['nick'], $_POST['pass']);
      $err = 'Nickname or password incorrect';
  }
  

• the same goes for the logout code

To be honest, in your place I would rather try to learn how to make a more organized procedural code. For example instead of such "accordion" as in your add.php why not to make it more level

$err = [];
$inputValidator = new InputValidator($name);
if (!$inputValidator->length(1, 50)) {
    $err[] = 'File name length is not correct';
}
$inputValidator = new InputValidator($content);
if (!$inputValidator->length(0, 10000)) {
    $err[] = 'Your file size is too big';
}
if (!$err) {
    require 'includes/db.php';
    $files = new Files($pdo, $_SESSION['userid']);
    $files->add($name, $content);
    $_SESSION['message'] = 'File has been added successfully';
    $_SESSION['filesData'] = null;
    $client->redirect('system.php');
}

And you also need learn the basic PHP syntax. For example, why $stmt->fetchAll()[0]['id']; when there is fetch(), or even fetchColumn() intended exactly for this purpose? Both fetchAll() and fetch() would raise an error when there is no such user found, where fetchColumn() will just return FALSE.

Oh - and you really, really should learn basic SQL as well.

 $stmt->bindValue(':id', rand(10000001,99999999) . rand(10000001,99999999));

is abaslutely not how it works. You've got to make the id field auto_increment PRIMARY KEY in the table, and it will be assigned automatically and without the risk of collisions.

Your idea on using exceptions is also wrong. As well as on the variable scope. To catch every exception on the spot is not how exceptions are intended to work. Least there is any point in assigning the error message to a variable that will be discarded the next instant. In PHP, a variable inside a function is only alive a long as the function is executed and when the function is finished your $err variable gets in to the void. And therefore it makes no sense to check if (!isset($err)) - it is never set. In general, you should leave error and exceptions alone, and handle them in a single place with a dedicated code, instead of littering your application code with thousands of catch blocks. Here is an article I wrote on PHP error reporting.

Don't be discouraged though. OOP is a hard tack by itself, but tenfold when compared to obscene simplicity of traditional PHP. OOP is not a syntax. It's a mindset. No wonder you don't know what an abstract class is for, simply because you don't know yet what a regular class is for either. It takes time to realize.

To me, OOP is an approach that improves the manageability of the code at the cost of adding extra complexity. Which means that as long as your procedural code is manageable, you won't see any benefit from OOP, yet your code will become unnecessarily more complex.