PHP classes with functions used for building a web application

Question

The following class is set of functions I use for web development(examples below) and I was wondering how to structure it.

Examples:

//Checking if user isn't admin so that he can access login page
$helper = new Base();
if ($helper->is_admin()) {
    $helper->location("index.php");
}

//Encoding output to prevent XSS
$html = "<script>alert("XSS")</script>";
echo "<h1>". $helper->clean_html($html) ."</h1>";

//Redirect to another webpage and exit
$helper->location("error.php");

Is having just one class for all simple functions the correct way?

<?php
class Base
{
    public function __construct()
    {
        session_start();
    }

    public function location($dir = "index.php")
    {
        header("Location: ".$dir);
        exit();
    }

    public function is_logged_in() {
        return (isset($_SESSION['logged_in']) && $_SESSION['logged_in']);
    }

    public function is_admin() {
        return (isset($_SESSION['admin']) && $_SESSION['admin']);
    }

    /*
     * Clean functions para prevenir XSS
     */
    public function clean_html($html) {
        return htmlspecialchars($html, ENT_QUOTES, 'utf-8');
    }

    public function clean_json($json) {
        return json_encode($json, JSON_HEX_QUOT|JSON_HEX_TAG|JSON_HEX_AMP|JSON_HEX_APOS);
    }

    /*
     * Check functions
     */
    public function check_token($token, $dir)
    {
        if ($token != $_SESSION["csrf_token"]) {
            $this->location($dir);
        }
    }

    public function check_login($dir)
    {
        if (!$this->is_logged_in()) {
            $this->location($dir);
        }
    }

    public function check_admin($dir)
    {
        if (!$this->is_admin()) {
            $this->location($dir);
        }
    }

    public function check_input($required, $erro)
    {
        foreach ($required as $field) {
            if (!empty($_POST[$field])) {
                $this->location($erro);
            }
        }
    }
}

Or since the check functions build on previous functions should I structure it the following way:
Base class:

<?php
class Base
{
    public function __construct()
    {
        session_start();
    }

    public function location($dir = "index.php")
    {
        header("Location: ".$dir);
        exit();
    }

    public function is_logged_in() {
        return (isset($_SESSION['logged_in']) && $_SESSION['logged_in']);
    }

    public function is_admin() {
        return (isset($_SESSION['admin']) && $_SESSION['admin']);
    }

    /*
     * Clean functions para prevenir XSS
     */
    public function clean_html($html) {
        return htmlspecialchars($html, ENT_QUOTES, 'utf-8');
    }

    public function clean_json($json) {
        return json_encode($json, JSON_HEX_QUOT|JSON_HEX_TAG|JSON_HEX_AMP|JSON_HEX_APOS);
    }
}

And
Helper class:

<?php
class Helper extends Base
{
    protected $base;

    public function __construct()
    {
        $this->base = new Base;
    }

    public function check_token($token, $dir)
    {
        if ($token != $_SESSION["csrf_token"]) {
            $this->base->location($dir);
        }
    }

    public function check_login($dir)
    {
        if (!$this->base->is_logged_in()) {
            $this->base->location($dir);
        }
    }

    public function check_admin($dir)
    {
        if (!$this->base->is_admin()) {
            $this->base->location($dir);
        }
    }

    public function check_input($required, $erro)
    {
        foreach ($required as $field) {
            if (!empty($_POST[$field])) {
                $this->base->location($erro);
            }
        }
    }
}

Second Version made after reading the comments What can I do to improve it further

Base:

/*
 * Miscellaneous functions
 */
class Base
{
    public static function location($dir = "index.php")
    {
        header("Location: ".$dir);
        exit();
    }

    public static function check_input($required, $error)
    {
        foreach ($required as $field) {
            if (empty($_POST[$field])) {
                Base::location($error);
            }
        }
    }
}

Session:

/*
 * Session handling class
 */
class Session
{
    public function __construct()
    {
        session_start();
    }

    public function initialize_user_session($admin, $user_id) {
        $_SESSION["admin"] = $admin;
        $_SESSION["loggedIn"] = true;
        $_SESSION["user_id"] = $user_id;
        $_SESSION["csrf_token"] = bin2hex(random_bytes(32));
    }

    public function logout(){
        session_destroy();
        exit();
    }

    public function is_logged_in() {
        return (!empty($_SESSION['logged_in']));
    }

    public function is_admin() {
        return (!empty($_SESSION['admin']));
    }

    /*
     * Check functions
     */
    public function check_token($token, $dir)
    {
        if ($token != $_SESSION["csrf_token"]) {
            Base::location($dir);
        }
    }

    public function check_login($dir)
    {
        if (empty($_SESSION['logged_in'])) {
            Base::location($dir);
        }
    }

    public function check_admin($dir)
    {
        if (empty($_SESSION['admin'])) {
            Base::location($dir);
        }
    }
}

Inpu_Encoding:

/*
 * Functions to prevent XSS
 */
class Input_Encoding
{
    public static function clean_html($html) {
        return htmlspecialchars($html, ENT_QUOTES, 'utf-8');
    }

    public static function clean_json($json) {
        return json_encode($json, JSON_HEX_QUOT|JSON_HEX_TAG|JSON_HEX_AMP|JSON_HEX_APOS);
    }
}

Answer 1

If you let me to be frank, this is not a class but rather a ratatouille - that is a random collection of functions.

I do understand your idea and a chain of thoughts and in a way you are trying to do the right thing - to achieve the main goal of every programmer - to reduce the amount of code written. And it's for the good you started with OOP.

However, there are common pitfalls on this road, and you didn't miss any of them.

Ironically, the most natural part of OOP - inheritance - should be avoided as much as possible. It's a very dangerous practice that will lead to spaghetti code despite being OOP.

Instead, the first OOP rule you should learn and implement is the Single responsibility principle. As soon as you grasp it, you will see that your lass is an Irish stew consisting of every task your program is about to perform. This is not OOP.

Although I understand your intention to have helper methods in all your classes, it is not an excuse for having such a mess. But there is a solution called "composition over inheritance". If you need some service, it has to be injected in your class, not inherited from a parent.

So, now I can tell that you started to move into the right direction. But still such a decomposition you did already is not enough:

• There are functions related to processing the user input - they should go into a distinct class
• There are functions related to user user authorization - they should go into a distinct class
• There are functions related to HTML/JSON output - they should go into a distinct class

In the end there will be no Base class but several other classes each related to its particular niche.

Answer 2

Or since the check functions build on previous functions should I structure it the following way

This is not a case that makes you go for inheritance. You should go for inheritance to solve the problem of "repeating same properties in more than 1 class", for example, you have 2 classes Lion and Cheetah

class Lion{
    protected $speed;
    protected $age;
    protected $home;
    protected $sex;
    protected $preferredPrey;

    protected $maneColor;
} 

class Cheetah{
    protected $speed;
    protected $age;
    protected $home;
    protected $sex;
    protected $preferredPrey;

    protected $eyeStripesThickness;
} 

Instead of doing that you go with this:

class Predator{
    protected $speed;
    protected $age;
    protected $home;
    protected $sex;
    protected $preferredPrey;
} 
class Lion extends Predator{
    protected $maneColor;
}
class Cheetah extends Predator{
    protected $eyeStripesThickness;
}

As you see, this solved the problem of repeating the same properties in the classes, and of course to repeat the same changes you made to a property that is shared between the Lion and Cheetah, now if you need to add, remove, change a property between Lion and Cheetah do it in only 1 place i.e. "The parent class". That makes your code crazy-easier to maintain and organized. Think of:

Animal
Animal>Predator
Animal>prey
Animal>marine
Animal>Predator>Lion
Animal>Predator>Cheetah
Animal>prey>gazelle
Animal>prey>Goat
Animal>marine>Dolphin

Back to your question, Base or Base and Helper ?

Do you have, or will have another Class that will extend Base other than Helper? I guess "no" , so there is no need for this Helper class.

Other notes on the code

• As said by the earlier answer this is just a class that groups some function that you need to use in your projects to reduce your coding (your own framework), it has no properties, just a group of random functions.

• The functions that don't need the object, better to be static, so you can call them without creating the object and calling it's __construct function, for example location, clean_html , clean_json all don't depend on the object, so make them static so you can call them without creating the object - e.g.

  public static function clean_html($html) {
      return htmlspecialchars($html, ENT_QUOTES, 'utf-8');
  }
  

• This

  isset($_SESSION['admin']) && $_SESSION['admin'])
  

  can be replaced with this

  !empty($_SESSION['admin'])
  

• The class is using $_SESSION['admin'] and $_SESSION['logged_in'] but it doesn't set them. It's better to also include functions that set these variables in this class, so the maintainer of your class in the future (you or someone else) can edit the class without depending on the outside world of the class, make it self-contained and ask yourself

  "If I edited this class later will I have to go outside the class to check something ?"

Answer 3

Here is another thing I noticed about your code:

If $helper is not the admin, you did not specify what will happen so I suggest that you modify it like this:

if($helper->is_admin()) {
          go to admin panel
} else {
      go to normal user panel
}