Intranet PHP application

Question

I'm more of an IT guy (no CS course) with a strong and passionate relationship with Unix and I love KISS.

I'm writing an application to help my coworkers with their daily tasks. Every now and then I get request from coworkers to add functionality to help them. Most of the time I will turn an Excel spreadsheet to a form based "module" in my application.

The application is built with PHP and runs on Apache, using MultiViews for "pretty-urls" and FallbackResource /index.php.

The folder structure looks like this:

app/
    _views/
        _errors/
            401.php
            403.php
            404.php
        user/
            add.php
            get.php
    core/
        config.php
        user.php
    hr/
        department.php
    .settings.php
    bootstrap.php
lib/
    z/
        form.php
        route.php
        view.php
logs/
public/
    css/
    fonts/
    images/
    js/
    index.php
    user.php

app/bootstrap.php is prepended to every .php files in the public/ folder.

<?php
// autoloader
set_include_path(implode(PATH_SEPARATOR, array(
    get_include_path(),
    dirname(__DIR__) . DIRECTORY_SEPARATOR .'lib',
    __DIR__,
)));
spl_autoload_extensions('.php');
spl_autoload_register();

// application user stuff
session_start();
if (isset($_SESSION['username'])) {
    $user = new core\user($_SESSION['username']);
    define('LOGGED', true);
}
else {
    $user = new core\user();
    define('LOGGED', false);
}

function http_error($error) {
    http_response_code($error);
    $view = new z\view('_errors/'. $error .'.php');

    die($view);
}

My controllers are in the public/ folder and look like this:

<?php # /user.php ; controllers for /user(.*)
use z\form;
use z\route as main;
use z\view;

main::get('/user/add', function() use ($user) {
    if (!LOGGED)
            http_error(401);

    if (!$user->allowed('user-add'))
            http_error(403);

    $view = new view('user/add.php');

    die($view);
});

main::get('/user/%s', function($username) use ($user) {
    $person = new core\user($username);

    if ($person->notfound === true)
            http_error(404);

    if (!LOGGED)
            http_error(401);

    if (!$user->allowed('user-view'))
            http_error(403);

    $view = new view('user/get.php');
    $view->person = $person;

    if ($user->allowed('user-manage')) {
        $view->procedures = $person->procedures();
        $departments = hr\department::select();
        array_shift($departments);
        $view->departments = $departments;
    }

    die($view);
});

main::post('/user', function() use ($user) {
    if (!LOGGED)
            http_error(401);

    if (!$user->allowed('user-add'))
            http_error(403);

    $form = new form($_POST);
    if (core\user::insert(array(
        'value1' => $form->field1->value,
        'value2' => $form->field2->value,
        ...
    ))) {
        main::redirect('/user/'. $form->username->value);
    }
    else {
        $view = new view('user/add.php');
        $view->error = true;
        $view->message = 'Error message';

        die($view);
    }
});

...

http_error(404);

View 'user/get.php':

<?php $layout = 'html.php' ?>
<h1><?= $person->displayname ?></h1>
<p><?= $person->title ?></p>
<ul>
    <li>Office: <?= $person->phone_office ?></li>
    <li>Mobile: <?= $person->phone_mobile ?></li>
</ul>
<div class="tabs">
    <ul class="tabs-nav">
        <?php if ($user->allowed('user-manage-somestuff')): ?>
        <li><a href="#tab-1">Tab 1</a></li>
        <?php endif ?>

        <?php if ($user->allowed('user-manage-someotherstuff')): ?>
        <li><a href="#tab-2">Tab 2</a></li>
        <?php endif ?>

        <li><a href="#tab-3">Tab 3</a></li>
    </ul>

    <?php if ($user->allowed('user-manage-somestuff')): ?>
    <div id="tab-1"></div>
    <?php endif ?>

    <?php if ($user->allowed('user-manage-someotherstuff')): ?>
    <div id="tab-2"></div>
    <?php endif ?>

    <div id="tab-3"></div>
</div>

This is then wrapped in an 'html.php' layout by the view class.

The core\config class:

<?php
namespace core;

class config
{
    private static
        $__settings = false;

    protected function __construct() {}

    public static function get($key) {
        if (!self::$__settings)
            self::$__settings = include '../.settings.php';

        $context = self::$__settings;
        $pieces = explode('.', $key);

        foreach ($pieces as $piece) {
            if (!is_array($context) || !array_key_exists($piece, $context))
                return false;

                $context = &$context[$piece];
            }

        return $context;
    }
}

The core\user class:

<?php
namespace core;

use z\db;

class user
{
    public
        $displayname,
        $firstname,
        $lastname,
        $email,
        $phone_office,
        $phone_mobile,
        $notfound = true;

    protected
        $_roles = [];

    public function __construct($username = 'anonymous') {
        $this->displayname = 'Guest';
        $this->username = $username;
        if ($this->username != 'anonymous')
            $this->__init();
    }

    protected function __init() {
        $db = db::instance(config::get('db'));
        $stmt = $db->prepare("
            SELECT * FROM users WHERE username = :username");
        $stmt->execute(array(
            ':username' => $this->username
        ));
        $rslt = $stmt->fetch();
        if (!$rslt)
            return false;

        $this->firstname = $rslt['firstname'];
        $this->lastname = $rslt['lastname'];
        $this->displayname = $this->firstname .' '. $this->lastname;
        ...
    }

    public static function insert(array $user) {
        $db = db::instance(config::get('db'));
        $stmt = $db->prepare("
            INSERT INTO users(field1, ...)
            VALUES (:value1, ...)");

        return $stmt->execute(array(
            ':value1' => $user['field1'],
            ...
        ));
    }

    ...
}

The hr\department class:

<?php
namespace hr;

use core\config;
use z\db;

class department
{
    public static function insert(array $department) {...}

    public static function select($id = -1) {...}

    public static function update(array $department) {...}

    public static function delete($id) {...}
}

As the application is getting bigger, I would like some advice on my code. Can you see problems with the way I'm doing things?

Is this code "maintainable"? I wouldn't want my application to be removed if/when I leave and let my users in pain. My main concern here is if the guy who will replace me be able to understand and work with this code "easily". Note that I removed all code comments before posting but everything is documented in comments.

I'm also having a lot of difficulties trying to understand tests and how to write them, can someone explain to me how can I write tests for my application? and will I really benefit from writing/using them?

I spent a lot of time looking at PHP frameworks from Zend to Symphony, Kohana, CI, Laravel, Phalcon and many many others... I really liked some of the "micro-frameworks" like Slim and Lemonade but the file structure didn't make sense to me, why have such a complicated file structure and so many files/folders for such a simple thing?

Then I found a small PHP library web.php and took it's route, view and form class/functions as a starting point to build my application.

About the view class, I can use a different layout from inside a controller like this:

$view = new view('view.php', 'layout.php');

Answer 1

I really like the current ideas. Following Unix and Kiss using existing solutions like MultiViews.

Everything I can see looks okay to me. Except that it's not the same way as done in most frameworks there days. So the only thing I would recommend for now it so provide a readme.md pointing to the differences most developers would indent to find.

E.g. document that Backend-Code (Controllers) are inside the public folder. Document that there is no single entry point like one index.php but you are using MultiViews and prepending app/bootstrap.php to all php files. Such things.

Beside that, you can follow PSR-2 (www.php-fig.org) as Coding Guideline, so this is already documented and PHP Developers familiar with PSR-2 won't struggle with code formatting.

For your config class, I would reduce the logic inside the get method to keep it short. Read in configuration inside the construct and parse it within your get. Use recursion to walk deeper in the hierarchy, instead of using a reference you change in each iteration.

I would recommend small parts instead of a framework for your work. So e.g. search on packagist.org for ready to use components. For your Models, I would recommend the ActiveRecord Design Pattern (en.wikipedia.org/wiki/Active_record_pattern), which is already partially implemented by yourself. Currently I'm using ActiveRecord to keep models slim and don't waste time with DB implementations.

Answer 2

I wouldn't stress about tests if it's a smaller company, an internal application, and will not be distributed.

Generally a framework is a good idea. It gives you everything that you're asking for.

I'd recommend Yii for performance, but Cake because of its popularity, which makes it easily maintainable. So the next guy might already know it, and the documentation of the framework minimises your need to document. i.e. If he needs to do something he can just look on their site.

The simplest and most effective way forward would be to move it into Cake.

Was this application built proactively? Is it essential that you concern yourself with the future of this application?

After looking at the code:

There are some important structural problems in this application.

First off you need to read up on PHP best practices. You shouldn't use short tags.

You don't need the user and department classes. You should have a generic model class that will deal with these models, which can be extended if you need to do something special for a certain model. i.e. Object oriented with inheritance.

I see the $_POST goes straight into your form class. Does it get cleaned in there?

You are setting your layout in the view. What if you were doing something server side, such as browser detection, and then wanted to serve a different layout? You should set your layout where the logic is. In the controller.

Definitely move this into a common and accessible framework. Cake or Code Igniter. I think Yii maybe a bit too steep of a learning curve.

Also, because it is effectively business software, you will benefit from improved security within a framework.