2
\$\begingroup\$

I'd appreciate some feedback on my latest PHP script. It is supposed to work like this:

  1. When obtained, it checks whether the cache is still up-to-date
  2. IF YES, return the cache; IF NOT, reset the cache
  3. Get all JS scripts in the parent directory
  4. Minimize the code
  5. Add it to the cache
  6. Return the cache

// Script combines & minimizes all .js files in the parent folder
// Result is cached until a change to the files is dealt

header('Content-type: application/x-javascript');

$cache_path = 'cache.txt';
$index_path = 'cache-index.txt';

function getScriptsInDirectory(){
    $array = Array();
    $scripts_in_directory = scandir('.');
    foreach ($scripts_in_directory as $script_name) {
        if (preg_match('/(.+)\.js/', $script_name))
        {
            array_push($array, $script_name);
        }
    }
    return $array;
}

function compilingRequired(){
    global $cache_path;
    global $index_path;
    if (file_exists($cache_path) && file_exists($index_path))
    {
        $cache_time = filemtime($cache_path);
        $files = getScriptsInDirectory();
        foreach ($files as $script_name) {
            if(filemtime($script_name) > $cache_time)
            {
                return true;
            }
        }

        $array = explode(PHP_EOL, file_get_contents($index_path));
        foreach($array as $name)
        {
            if (!file_exists($name))
            {
                return true;
            }
        }

        return false;
    }
    return true;
}

function compressScript($buffer) {
    $buffer = preg_replace('!/\*[^*]*\*+([^/][^*]*\*+)*/!', '', $buffer);
    $buffer = str_replace(array("\r\n","\r","\n","\t",'  ','    ','     '), '', $buffer);
    $buffer = preg_replace(array('(( )+{)','({( )+)'), '{', $buffer);
    $buffer = preg_replace(array('(( )+})','(}( )+)','(;( )*})'), '}', $buffer);
    $buffer = preg_replace(array('(;( )+)','(( )+;)'), ';', $buffer);
    return $buffer;
}

if (compilingRequired())
{
    if (file_exists($cache_path)){
        unlink($cache_path);    
        unlink($index_path);
    }

    $scripts_in_directory = getScriptsInDirectory();
    $file_handler = fopen($cache_path, 'w+');
    $cache_handler = fopen($index_path, 'w+');

    foreach ($scripts_in_directory as $name)
    {
        if (strlen(file_get_contents($cache_path)) > 0){
            fwrite($file_handler, str_repeat(PHP_EOL, 2));
        }

        fwrite($file_handler, '/**** ' . $name . ' ****/' . str_repeat(PHP_EOL, 2));
        fwrite($file_handler, compressScript(file_get_contents($name)));
        fwrite($cache_handler, $name . PHP_EOL);
    }

    echo file_get_contents($cache_path);
}
else
{
    echo file_get_contents($cache_path);
}
\$\endgroup\$
2
  • 2
    \$\begingroup\$ If you are able to use a more recent version of PHP, try refactoring this as OOP, and try functions like glob instead of scandir, they're cleaner. \$\endgroup\$ Commented Sep 11, 2013 at 16:33
  • \$\begingroup\$ @MMiller - or even SPL (Directory, DirectoryIterator, RecursiveDirectoryIterator) if they can refactor to OOP on a newer PHP version \$\endgroup\$ Commented Sep 11, 2013 at 19:05

1 Answer 1

1
\$\begingroup\$

This snippet at the end...

if (compilingRequired())
{
    if (file_exists($cache_path)){
        unlink($cache_path);    
        unlink($index_path);
    }

    $scripts_in_directory = getScriptsInDirectory();
    $file_handler = fopen($cache_path, 'w+');
    $cache_handler = fopen($index_path, 'w+');

    foreach ($scripts_in_directory as $name)
    {
        if (strlen(file_get_contents($cache_path)) > 0){
            fwrite($file_handler, str_repeat(PHP_EOL, 2));
        }

        fwrite($file_handler, '/**** ' . $name . ' ****/' . str_repeat(PHP_EOL, 2));
        fwrite($file_handler, compressScript(file_get_contents($name)));
        fwrite($cache_handler, $name . PHP_EOL);
    }

    echo file_get_contents($cache_path);
}
else
{
    echo file_get_contents($cache_path);
}

If they both end the same, there's really no need for an else case, you can just put the statement after the if statement.

if (compilingRequired())
{
    if (file_exists($cache_path)){
        unlink($cache_path);    
        unlink($index_path);
    }

    $scripts_in_directory = getScriptsInDirectory();
    $file_handler = fopen($cache_path, 'w+');
    $cache_handler = fopen($index_path, 'w+');

    foreach ($scripts_in_directory as $name)
    {
        if (strlen(file_get_contents($cache_path)) > 0){
            fwrite($file_handler, str_repeat(PHP_EOL, 2));
        }

        fwrite($file_handler, '/**** ' . $name . ' ****/' . str_repeat(PHP_EOL, 2));
        fwrite($file_handler, compressScript(file_get_contents($name)));
        fwrite($cache_handler, $name . PHP_EOL);
    }
}
echo file_get_contents($cache_path);

Ideally, though, it'd read something like

if (compilingRequired())
{
    compile();
}
printCompressedFiles();

Because you want to program on the same level of abstraction - high level code is in "programmer english", describing the actions that need to be done (basically that list of 6 steps of yours, maybe compressed a bit). Low level code is pretty much regular code - code actually doing the work.

\$\endgroup\$

Your Answer

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.