2
\$\begingroup\$

I wrote an PHP Autoloader to include it "easyer" to my projects.

Here the folder structure:

www
├───index.php
├───init.php
└───Libarys
    └───bootstrap
        ├───index.php
        ├───bootstrap_loader.php
        └───bootstrap_core_files (Files and Directories (Abbreviated so it doesn't get too much))

www/index.php:

It's just like the main method in Java, which starts on execution. This integrates the init.php file.

<?php
    include_once("./init.php");
    $init = new Initialisation();
    $init->loadBS();
?>

www/init.php:

It's just like the init method in Java, which initialized integrate following things:

Directory Locations (initialized)
BootStrapLoader (integrate (www/Libarys/bootstrap/index.php)
<?php
    class Initialisation {
        
        private $direct;
        
        function getFileLocation() {
            $value = "";
            $value = $_SERVER["REQUEST_URI"];
            return($value);
        }
        function getCurrentDirectory() {
            $dir = "";
            $getFileLocation = $this->getFileLocation();
            $countetSlashes = substr_count($getFileLocation,"/")-1;
            
            if ($getFileLocation !== "") {
                for ($i = 0; $i < $countetSlashes; $i++) {
                    $dir = $dir."../";
                }
            }
            $dir = $dir."./Libarys/bootstrap";
            return $dir;
        }
        function setDirect($dir) {
            $this->direct = $dir;
        }
        function getDirect() {
            return $this->direct;
        }
        function loadBS() {
            $this->setDirect($this->getCurrentDirectory());
            include_once($this->getDirect()."/index.php");
            $loadBS = new LoadBootStrap();
            $loadBS->loadBootStrap();
        }
    }
?>

www/Libarys/bootstrap/index.php:

This index.php file just starts the true loader (www/Libarys/bootstrap/bootstrap_loader.php).

<?php
    class LoadBootStrap {
        
        function loadBootStrap() {
            $init = new Initialisation();
            $init->setDirect($init->getCurrentDirectory());
            include_once($init->getDirect()."/bootstrap_loader.php");
            $bootstrap = new BootStrap();
            $bootstrap->loadFileGeter("JS", $init->getDirect());
            $bootstrap->loadFileGeter("CSS", $init->getDirect());
        }
    }
?>

www/Libarys/bootstrap/bootstrap_loader.php:

The bootstrap_loader.php searches and includes the CSS and JS files from the BootStrap Framework to the www/index.php file. I tested those scripts in more than 4 sub directories with sub directories and they worked well.

<?php
    class BootStrap {
        function loadFileGeter($param, $direct) {
            switch ($param) {
                case "CSS":
                    $this->getFileList($direct."/".strtolower($param), $param);
                break;
                case "JS":
                    $this->getFileList($direct."/".strtolower($param), $param);
                break;
            }
        }
        function getFileList($dir, $param) {
            $dir = $dir;
            $files  = scandir($dir, 1);
            sort($files);
            foreach ($files as &$value) {
                if (str_ends_with($value,".".strtolower($param))) {
                    switch ($param) {
                        case "CSS":
                            $string = '<link rel="stylesheet" type="text/css" href="'.$dir.'/'.$value.'" />'."\n";
                            ECHO($string);
                        break;
                        case "JS":
                            $string = '<script async="" src="'.$dir.'/'.$value.'"></script>'."\n";
                            ECHO($string);
                        break;
                    }
                }
            }
        }
    }
?>
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Spacing could be better

Overall the code isn't difficult to read. Idiomatic PHP has spacing conventions, mainly outlined in the PHP Standards Recommendations - e.g. PSR-12: Extended Coding Style. There is one convention for spacing not followed by the code - and that is spacing around binary operators like . - e.g.

$dir = $dir."./Libarys/bootstrap";

For readability, it is better to have a space around all binary operators including the concatenation operator:

$dir = $dir . "./Libarys/bootstrap";

That way it is easier to see the placement of the operators and operands.

Naming can be confusing

The class Initialisation has a property $direct. In English direct is a verb or adverb but not really a noun, though it seems for the purposes of the code it stores a directory path. Perhaps a better name might be directory or else abbreviated as dir.

Some variable declarations become superfluous when overwritten

Looking at the first method in the class Initialisation - i.e. getFileLocation() I see:

function getFileLocation() {
    $value = "";
    $value = $_SERVER["REQUEST_URI"];
    return($value);
} 

The first line assigns an empty string to $value. The second line re-writes that variable with the request URI from the superglobal. Thus the first line becomes useless and it can be removed. Furthermore, $value is never altered or used in-between that last assignment and when it is returned, so it can be eliminated. The method can simply return $_SERVER["REQUEST_URI"].

In a previous review Dharman stated:

  • Don't put parentheses after echo. It's not a function and these parentheses are extremely confusing. Same applies to require

The same applies to return - parentheses are not needed for language constructs. As the documentation states:

These words have special meaning in PHP. Some of them represent things which look like functions, some look like constants, and so on - but they're not, really: they are language constructs.

1

The List of Keywords also includes the include_once statement and that statement appears in the loadBS() method with parentheses- those can be removed there as well.

Use String Concatenation assignment operators

In the next method of class Initialisation - i.e. getCurrentDirectory() the variable $dir is assigned to multiple times, often with a string literal appended to it - e.g.

 $dir = $dir."../"; 

and

 $dir = $dir."./Libarys/bootstrap";

Many high-level programming languages, including PHP, offer a string concatenation assignment operator. For PHP it is .=. The two lines cited above can be re-written as:

 $dir .= "../"; 

and

 $dir .= "./Libarys/bootstrap";

Loop can be replaced with function call

PHP offers the built-in function str_repeat() which can be used to replace the for loop within Initialisation::getCurrentDirectory() repeated $countetSlashes - 1 times.

Redundant switch cases can be consolidated

The method Bootstrap::loadFileGeter() (which likely has a typo - typically it is written getter) has two case statements inside the switch:

class BootStrap {
        function loadFileGeter($param, $direct) {
            switch ($param) {
                case "CSS":
                    $this->getFileList($direct."/".strtolower($param), $param);
                break;
                case "JS":
                    $this->getFileList($direct."/".strtolower($param), $param);
                break;
            }
        }

In both cases the same method is called with the same arguments. Those can be consolidated:

            switch ($param) {
                case "CSS":
                case "JS":
                    $this->getFileList($direct."/".strtolower($param), $param);
                break;
            }

Going a step further, the switch statement could be replaced by a conditional block utilizing the PHP function in_array():

if (in_array($param, ['CSS', 'JS'])) {
    $this->getFileList($direct."/".strtolower($param), $param);
}

Then there is no need to add the case and break lines.

The method getFileList() also has a similar switch statement though the generated HTML tags are different. One could simplify that slightly by using a method to get the tag based on the value of $param. The method would be more granular, allowing for the code to be more in line with the Single Responsibility Principle.

Reference operator in loop

In that method getFileList() the foreach loop has a by-reference operator before $value which means it will be assigned by reference. 3

foreach ($files as &$value) {

However, it doesn't appear that there is a need to assign that variable by reference.

Closing tags can be omitted

PSR-12 also mentions:

The closing ?> tag MUST be omitted from files containing only PHP.

Per the PHP documentation for PHP tags:

If a file contains only PHP code, it is preferable to omit the PHP closing tag at the end of the file. This prevents accidental whitespace or new lines being added after the PHP closing tag, which may cause unwanted effects because PHP will start output buffering when there is no intention from the programmer to send any output at that point in the script.

Sorting may not be necessary

In the method BootStrap:: getFileList() there is a call to sort($files). Maybe you prefer to have the files sorted in the HTML output but that doesn't appear to be a requirement. It could be removed, though if it is preferable to have them sorted by name then consider using natsort() for "natural order" sorting - e.g. for sheet2.css after sheet12.css.

Files could be filtered to certain extensions

One could use the [glob()] function to find files with a pattern - e.g. *.css, *.js - then the amount of code to filter files could be decreased.

\$\endgroup\$
3
  • \$\begingroup\$ Should sort() be natsort()? Is it needed at all? glob() allows limited pattern filtering to only return certain suffixes. Food for thought. \$\endgroup\$ Commented Dec 10, 2022 at 11:40
  • \$\begingroup\$ I'll take a closer look later and try to implement them according to the "CleanCode" rules. By the way, it's my first "real" PHP OOP "project". \$\endgroup\$
    – NvrKill
    Commented Dec 12, 2022 at 11:28
  • 1
    \$\begingroup\$ @mickmackusa thanks for the yummy food for thought. I have added a couple more sections \$\endgroup\$ Commented Dec 14, 2022 at 23:58

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.