Thread Rating:
  • 0 Vote(s) - 0 Average
  • 1
  • 2
  • 3
  • 4
  • 5
Code Refactor of function in session.inc
03-26-2014, 06:30 PM,
#1
Code Refactor of function in session.inc
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.
Reply
03-26-2014, 07:07 PM,
#2
RE: Code Refactor of function in session.inc
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
Reply


Forum Jump:


Users browsing this thread: 1 Guest(s)