There's a delicate balance between useless instance variables and extracting well-named variables for readability.

LongController.php

<?php

class ModuleController extends Controller
{
    public function getIconAction($moduleId)
    {
        $moduleFinder = $this->get('module_finder');

        $module = $moduleFinder->getModule($moduleId);
        if (!$module) {
            return new Response('Could not find given moduleId', 404);
        }

        $logoPath = $module->getModuleFilePath('Resources/images/logo.png');

        // Return the module's logo, if one exists
        if (file_exists($logoPath)) {
            $image = file_get_contents($logoPath);
        }
        // Fallback to default logo
        else {
            $image = file_get_contents(__DIR__ . '/../Resources/images/dashlogo-small.png');
        }

        return new Response($image, 200, ['Content-Type' => 'image/png']);
    }
}

ShorterController.php

<?php

class ModuleController extends Controller
{
    public function getIconAction($moduleId)
    {
        $module = $this->get('module_finder')->getModule($moduleId);
        if (!$module) {
            return new Response('Could not find given Module for given moduleId', 404);
        }

        // Return the module's logo, if one exists
        // else fallback default
        $logoPath = $module->getModuleFilePath('Resources/images/logo.png');
        if (!file_exists($logoPath)) {
            $logoPath = __DIR__ . '/../Resources/images/dashlogo-small.png';
        }

        return new Response(file_get_contents($logoPath), 200, ['Content-Type' => 'image/png']);
    }
}

Reading the first LongController example, notice it has two instance variables that have no effect on the actual output of the function: $moduleFinder and $image.

We only use $moduleFinder once after it is defined, yet it sticks around and is available until the method returns a value at the bottom.

We only use $image in the final returned object (and it's defined in two places with duplicate code.) $image is never used again after being defined except in the final return new Response. We aren't doing any further checking on $image other than handing it off to the Response object.

You could argue that "new Response()" is too long on one line and a bit messy with 3 parameters, so you want to extract each Response parameter to its own variable for readability. I wouldn't argue too hard over that, but its current length is within my personal threshold and everything having to do with what data is being returned with the Response is on one line.

I think this idea of removing excessive instance variables is so someone reading the code can FOCUS on the important part of the code: the logic for HOW the image is located and then loaded.

Each variable your method defines is another value that could potentially change before the end of the method. Maybe you're like me: when reading a method I keep each variable in a mental registry that 1) knows the variable exists and 2) stores its current value. My mental register keeps state until I get to the return statement at the bottom of the function body.

When reading code and reading over newly defined variables I must ask myself, "Is this variable used anywhere else? Is it being mutated before the time I JUST NOW read it in the code and the last time I modified its value in my mental registry? Did I miss a mutation?" This extra cognitive load to keep all of this straight is ultimately useless in me figuring out exactly what or how a method is doing work.

Extra mentally juggling leaves room for bugs to creep into your code. You worry more about the state of the system and a tad bit less about the accuracy of what the code is doing. If we write less code and give it less places to change we leave less room for bugs to hide.

This is a good refactoring example!

When I'm writing methods, I like to start by declaring all the things I'll probably end up using. This gives me a nice block of variables at the top of the method that primes me for what to expect in the code that follows. It also has the benefit with phpstorm of letting me click on the variable names and see how they're used in the code below.

I tend to group the validation rules right next to the variable assignment, if possible, which is why you see the if (!$module) right under $module.

I totally agree with you about the redundant $image variable, that was a result of writing code at 5pm. :)

I like your refactored method much better, I just wanted to give you an overview of how I write/read code since I enjoyed seeing what your mental process was.