webERP Forum

Full Version: Code Refactor of function in session.inc
You're currently viewing a stripped down version of our content. View the full version with proper formatting.
The function findLogoFile in session.inc (lines 217 to 250) is horribly inefficient and it is not clear what it is actually doing.

The code:
Code:
function findLogoFile($CompanyDir, $PathPrefix) {
        $dir = $PathPrefix.'companies/' . $CompanyDir . '/';
        $DirHandle = dir($dir);
        while ($DirEntry = $DirHandle->read() ){
            if ($DirEntry != '.' AND $DirEntry !='..'){
                $InCompanyDir[] = $DirEntry; //make an array of all files under company directory
            }
        } //loop through list of files in the company directory
        if ($InCompanyDir !== FALSE) {
            foreach($InCompanyDir as $logofilename) {
                if (strncasecmp($logofilename,'logo.png',8) === 0 AND
                    is_readable($dir . $logofilename) AND
                    is_file($dir . $logofilename)) {
                    $logo = $logofilename;
                    break;
                }
            }
            if (!isset($logo)) {
                foreach($InCompanyDir as $logofilename) {
                    if (strncasecmp($logofilename,'logo.jpg',8) === 0 AND
                        is_readable($dir . $logofilename) AND
                        is_file($dir . $logofilename)) {
                        $logo = $logofilename;
                        break;
                    }
                }
            }
            if (empty($logo)) {
                return null;
            } else {
                return 'companies/' .$CompanyDir .'/'. $logo;
            }
        } //end listing of files under company directory is not empty
    }

What is it doing: It looks up the relevant company directory, and loops through every element in the directory and appends them to an array. It then goes looking for a file named exactly 'logo.png' in that array, tests if the file exists and is readable and is a file not a directory. If it can't find it, it loops through the directory array again but looking for 'logo.jpg'.

Replacement function:
Code:
function findLogoFile($CompanyDir, $PathPrefix) {
        $result = null;
        $dir = $PathPrefix . 'companies/' . $CompanyDir;
        if (file_exists($dir . '/logo.png')) {
            $result = 'companies/' . $CompanyDir . '/logo.png';
        } elseif (file_exists($dir . '/logo.jpg')) {
            $result = 'companies/' . $CompanyDir . '/logo.jpg';
        }
        return $result;
    }

Clearer, simpler, faster and more intuitive! If you wanted to be a little less intuitive but more careful, you could use
Code:
is_readable($dir . '/logo.png') && is_file($dir . '/logo.png')
instead of file_exists but the odds that logo.png exists but is not readable, or is a directory for some reason are probably so low the readability of file_exists would make it my preference.
Thanks serakfalcon. I think it was one of those functions that has been patched several times ending up in a mess. This one is much better.

Tim