Scrutinizer score optimization

Question

I started developping an image manipulation library and decided to go with Scrutinizer for the testing and inspection in one cool package but i'm not sure how to optimize my code to limit some of the stats that are degrading the overall score.

The code i'm showing you below has "Conditions: 6" which pushes it to "B" rating. There isn't anything special about the code, it checks for potential missing permissions or invalid directories and throw exceptions accordingly, so lowering the score there is quite hard without making the code react completely differently:

/**
 * Writes the image to a writable source
 *
 * @param int $compression Compression level 0-9, defaults to 9
 *
 * @throws \StandardExceptions\IOExceptions\FileNotWritableException
 * @throws \StandardExceptions\IOExceptions\DirectoryNotFoundException
 * @throws \StandardExceptions\IOExceptions\DirectoryNotWritableException
 */
public function write($compression = 9)
{
    $path = $this->getPath();
    $directory = $path->getPathInfo();
    if (file_exists($path->getPathname()) && !$path->isWritable()) {
        throw new FileNotWritableException();
    } elseif (!is_dir($directory->getPathname())) {
        throw new DirectoryNotFoundException();
    } elseif (is_dir($directory->getPathname()) && !$directory->isWritable()) {
        throw new DirectoryNotWritableException();
    }
    $options = [
        'png_compression_level' => $compression,
        'resolution-x' => $this->getDensity()->getDensity(),
        'resolution-y' => $this->getDensity()->getDensity(),
        'resolution-units' => $this->mapDensity($this->getDensity()),
    ];
    $this->getImage()->save($path, $options);
}

I don't understand why i have 6 conditions while there are only 3 conditions in there. And i don't understand how i could lower this!

score 0 · Answer 1 · answered Jul 14 '15 at 18:53

I tried to split my code into even more functions which now make the code less readable IMO and creates lots of useless functions in the class but at least it works and i was able to reduce the conditions to 1 or 2 in each block of code effectively bringing up my code to "A" everywhere.

/**
 * Writes the image to a writable source
 *
 * @param int $compression Compression level 0-9, defaults to 9
 */
public function write($compression = 9)
{
    $this->checkFileIsWritable();
    $this->checkDirectoryExists();
    $this->checkDirectoryIsWritable();
    $options = [
        'png_compression_level' => $compression,
        'resolution-x' => $this->getDensity()->getDensity(),
        'resolution-y' => $this->getDensity()->getDensity(),
        'resolution-units' => $this->mapDensity($this->getDensity()),
    ];
    $this->getImage()->save($this->getPath(), $options);
}

/**
 * Checks that the path is valid and throws exceptions
 * if there is something wrong
 *
 * @throws \StandardExceptions\IOExceptions\FileNotWritableException
 */
public function checkFileIsWritable()
{
    $path = $this->getPath();
    if (file_exists($path->getPathname()) && !$path->isWritable()) {
        throw new FileNotWritableException();
    }
}

/**
 * Checks that the path is valid and throws exceptions
 * if there is something wrong
 *
 * @throws \StandardExceptions\IOExceptions\DirectoryNotFoundException
 */
public function checkDirectoryExists()
{
    $path = $this->getPath();
    $directory = $path->getPathInfo();
    if (!is_dir($directory->getPathname())) {
        throw new DirectoryNotFoundException();
    }
}

/**
 * Checks that the is writable
 *
 * @throws \StandardExceptions\IOExceptions\DirectoryNotWritableException
 */
public function checkDirectoryIsWritable()
{
    $path = $this->getPath();
    $directory = $path->getPathInfo();
    if (is_dir($directory->getPathname()) && !$directory->isWritable()) {
        throw new DirectoryNotWritableException();
    }
}

The three lines you have now are guard clauses - and you can always split the three out into a new function - maybe something like `isFileWritable()`. Even with the lines still in there - it is more readable - but it's because you don't have to read the three new functions. — Alister Bulman, Jul 14 '15 at 19:23

Scrutinizer score optimization

1 Answers1