Simplified VarDumper in PHP

Question

I wrote a simplified VarDumper and wanted to get feedback on areas I can improve on my code. My biggest area of concern is the captureContext method making sure I get the correct file and line depending where dd() is called or if I call the dump method without helper function.

My Response class sets the http response status code.

Here is my helpers.php file

function dd(mixed ...$vars): void
{
    $response = new Response;
    $var_dumper = new VarDumper($response);

    if (array_key_exists(0, $vars) && 1 === count($vars)) {
        $var_dumper->dump($vars[0]);
    } else {
        foreach ($vars as $var) {
            $var_dumper->dump($var);
        }
    }

    exit(1);
}

and here is my VarDumper class

namespace App\Support\Debug\VarDumper;

use App\Http\Response;

class VarDumper
{
    private ?string $file_path = null;
    private ?int $line_number = null;
    private Response $response;

    public function __construct(Response $response)
    {
        $this->response = $response;
    }

    private function captureContext(): void
    {
        $trace = debug_backtrace(DEBUG_BACKTRACE_PROVIDE_OBJECT | DEBUG_BACKTRACE_IGNORE_ARGS, 20);
        $file = $trace[1]['file'];
        $line = $trace[1]['line'];

        for ($i = 2; $i < 20; ++$i) {
            if (isset($trace[$i]['function']) && 'dd' === $trace[$i]['function'])
            {
                $file = $trace[$i]['file'] ?? $file;
                $line = $trace[$i]['line'] ?? $line;
            }
        }

        $this->file_path = ltrim(str_replace(base_path(), '', $file), '/');
        $this->line_number = $line;
    }

    private function formatVar(mixed $var): false|string
    {
        ob_start();

        if (is_string($var) {
            echo "<div style="color: blue;">";
            echo $var;
            echo "</div>";
        }

        // check for int, float, boolean, arrays

        return ob_get_clean();
    }

    public function dump(mixed $var): void
    {
        $this->captureContext();
        $this->response->setStatusCode(500);

        $this->formatVar($var);
    }
}

Answer 1

A few suggestions as your code does a lot more than it needs to do. Sometimes it's just repeating something it doesn't need to (for example the captureContext() method is called for each variable).

I've commented the code along with the suggested edits...

class VarDumper
{
    private ?string $file_path = null;
    private ?int $line_number = null;
    private Response $response;

    public function __construct(Response $response)
    {
        $this->response = $response;
    }

    private function captureContext(): void
    {
        $backtrace = debug_backtrace(DEBUG_BACKTRACE_PROVIDE_OBJECT | DEBUG_BACKTRACE_IGNORE_ARGS, 20);
        $file = null;
        $line = null;

        // You don't know how long the stack is, so rather than for - use foreach
        foreach ($backtrace as $trace)  {
            // Wasn't sure if you wanted to say is NOT dd or is the dd function
            // This only picks up the file calling the dd (so ignores it)
            if ('dd' !== ($trace['function'] ?? ''))
            {
                $file = $trace['file'] ?? $file;
                $line = $trace['line'] ?? $line;
            }
            // As each loop will overwrite previous value until a non null value is
            // set, you can stop once both fields have a value?
            if ($file && $line) {
                break;
            }
        }

        // These don't seem to be used anywhere?
        $this->file_path = ltrim(str_replace(base_path(), '', $file), '/');
        $this->line_number = $line;
    }

    // Did this ever return false?
    private function formatVar(mixed $var): string
    {
        // If you are just building a string, there is no need to do output buffering.
        // Easier to just build the string.
        $output = '';

        if (is_string($var)) {
            // If this string could contain markup, then either include
            // <pre></pre> tags or use something like htmlentities()
            $output .= "<div style='color: blue;'>" . htmlentities($var) . "</div>";
        }

        // check for int, float, boolean, arrays

        return $output;
    }

    // Passed in an array of all of the variables
    public function dump(mixed $vars): string
    {
        // This is now only called once for the entire dd() call.
        $this->captureContext();
        $this->response->setStatusCode(500);

        // you just return the output and did nothing with it.
        // This builds a string of all of the variables output.
        $output = '';
        foreach ($vars as $var) {
            $output .= $this->formatVar($var);
        }
        
        // Now should set the body of the response to be $output
        // Depending on library?
        // Here it just returns it as a string.
        return $output;
    }
}

And the calling function now becomes...

function dd(mixed ...$vars): void
{
    $response = new Response;
    $var_dumper = new VarDumper($response);

    // As calling dump multiple times sets the response several times, 
    // it would probably be better to pass all of the data in at once.
    // This should instead deal with returning the response (which depends
    // on the framework or code you are using)
    echo $var_dumper->dump($vars);

    exit(1);
}

Answer 2

LGTM.

snapshots

Consider storing a private copy of values you output. Then on a subsequent call you can suppress duplicates, showing e.g. what changed on a certain iteration through a loop.

truncated output

It makes sense to suppress most of what could be a very large array. But it’s hard to properly interpret the output, as there is no ellipsis or other hint displayed.

Showing the total array length would offer a useful summary.

For a “small” array with a handful of elements, you might choose to display it in its entirety.

Consider adopting JSON compatibility as an explicit requirement for all formatted output.

test suite

This submission contains no automated unit tests, and it needs some.

CSS

The hard coded “blue” works, but it is inconvenient. Consider emitting a style sheet preamble, so “blue” appears just once in the output, making it easier to tailor colors as the need arises.

Answer 3

Redundant Length-Check

The if clause in this code is redundant:

    if (array_key_exists(0, $vars) && 1 === count($vars)) {
        $var_dumper->dump($vars[0]);
    } else {
        foreach ($vars as $var) {
            $var_dumper->dump($var);
        }
    }

Just use a single foreach() loop to run over the array.

Further Redundant Calls

If you call dd() with more than one parameter, you call captureContext() and setStatusCode(500) that many times as well, which is redundant. Possibly even worse, if you don't provide any arguments, neither of these two is called...

Magic Numbers

What are 2 and 20? I'm okay with the 500 HTTP status code, but those two deserve names and explanation.

Unused Returnvalue

In particular the one from formatVar() seems obvious. Please use a static analyzer (e.g. PHPStan or Psalm) to help you with those things. Also, use declare(strict_types=1) and a style checker.