5
\$\begingroup\$

I have this script to format var_export's result string. The information in the output is good, but I need know if the code performance is good or if it can be improved.

https://wtools.io/php-sandbox/biDk

<?php
#Variable:

$Test                            = [];
$Test['Check1']                  = null;
$Test['Check2']                  = [];
$Test['Check2']['int']           = 20;
$Test['Check2']['float']         = 20.35;
$Test['Check2']['string']        = 'Hello World';
$Test['Check2']['bolean']        = true;
$Test['Check2']['array']         = [];
$Test['Check2']['array']['data'] = 'Array Text';
class Example {
    function foo_function() {
        return "Hello World! Object";
    }
}
$var_object                         = new Example;
$Test['Check2']['array']['object']  = $var_object;
$Test['Check2']['array']['object2'] = $var_object->foo_function();

#Script Type:
function myGetType($var) {
    if (is_null($var) OR $var == 'null' OR $var == 'NULL') {
        return "(NULL)";
    }

    if (is_array($var)) {
        return "array";
    }

    if (in_array($var, array("true", "false"), true)) {
        return "boolean";
    }

    if ((int) $var == $var && is_numeric($var)) {
        return "integer" . '(' . strlen($var) . ')';
    }

    if ((float) $var == $var && is_numeric($var)) {
        return "float" . '(' . strlen($var) . ')';
    }

    if (is_object($var)) {
        return "object";
    }

    if (is_resource($var)) {
        return "resource";
    }

    if (is_string($var)) {
        return "string" . '(' . strlen($var) . ')';
    }

    return "unknown";
}

#Script Analisis:
function VarExportFormat($Var) {
    $textvar    = '';
    $textvar    = var_export($Var, true);
    $textvar    = preg_replace("/^([ ]*)(.*)/m", '$1$1$2', $textvar);
    $textvarArr = preg_split("/\r\n|\n|\r/", $textvar);
    # Analisis del tipo.
    foreach ($textvarArr as $key => $value) {
        preg_match('~=>\\s(.*?),~', $value, $newvalue);
        if (!empty($newvalue)) {
            $newvalue[1]      = str_replace("'", "", $newvalue[1]);
            $typeval          = myGetType($newvalue[1]);
            $value            = str_replace("=> ", "=> " . $typeval . ': ', $value);
            $textvarArr[$key] = $value;
        }
    }
    $textvarArr = preg_replace(["/\s*array\s\($/", "/\)(,)?$/", "/\s=>\s$/"], [NULL, ']$1', ' => array ['], $textvarArr);
    $textvar    = join(PHP_EOL, array_filter(["array ["] + $textvarArr));
    if (substr($textvar, -1) == '[') {
        $textvar = str_replace("[", "[]", $textvar);
    }
    $textvar = str_replace("__set_state", "__set_state(object)", $textvar);

    $textvar = highlight_string("<?php \n#output of Variable:\n" . $textvar . ";\n?>", true);
    return $textvar;
}

echo VarExportFormat($Test);
\$\endgroup\$
  • 1
    \$\begingroup\$ (string)$string == $string can be a replacement aswell for is_string(), same trick you did with int and floats.. \$\endgroup\$ – Raymond Nijland Oct 5 at 13:21
  • 1
    \$\begingroup\$ (object)$var == $var and (array)$var == $var can also works aswell to replace is_object and is_array but performance is more depending on the object or array size and or PHP version, this trick does not scale that that well as the int, string types.. Might be interesting to test with? \$\endgroup\$ – Raymond Nijland Oct 5 at 13:23
  • 1
    \$\begingroup\$ Using token_get_all() -> "token_get_all() parses the given source string into PHP language tokens using the Zend engine's lexical scanner. " seams to be making much more sense.. As you are making a parser/lexer color based syntax highlighter as var_export() returns parseable PHP... \$\endgroup\$ – Raymond Nijland Oct 5 at 13:35
  • 1
    \$\begingroup\$ @Raymond please do not abuse comments and break SE page design. If you want to provide a review, then post an answer. Comments should be used to request clarifications. Please remove all of your comments after posting your answer. \$\endgroup\$ – mickmackusa Oct 5 at 13:46
  • 1
    \$\begingroup\$ " Please remove all of your comments after posting your answer." @mickmackusa not going to place a answer/review as mine comments are heavy based on PHP version... Also comments are meant to possibly provide extra information where possible.. -> "Comments exist so that users can talk about questions and answers without posting new answers that do not make an attempt to answer the question asked. Comments are often used to ask for clarification on, suggest corrections to, and provide meta-information about posts. " \$\endgroup\$ – Raymond Nijland Oct 5 at 13:55
4
\$\begingroup\$

Your script is like a var_export()-var_dump() hybrid with flare (I've not come across highlight_string() before -- pretty cool).

  1. I ran some more tests with a few samples that I had laying around and noticed that your code is having some difficulties accessing the written "resource" branch. I tested with fopen($filename, 'r'); the var_export() evaluation of a resource is NULL and var_dump() returns resource(5) of type (stream). Because your script is founded on var_export(), it will fail to dignify the resource type data, I think you [sh/c]ould remove that from your battery of checks.

  2. Miscellaneous gotchas include:

    ['a => key' => 'something']
    

    and

    ['multiline' => 'line1
    line2']
    

    These will break your regex-based parsing -- of course, the former is going to be a much less common occurrence versus the latter.

  3. You can safely remove some useless declarations:

    • $Test = []; and $textvar = '';
  4. As a general rule, I prefer to never use OR or AND in my conditional statements as a matter of consistency and as a means to avoid fringe precedence issues.

  5. preg_replace("/^([ ]*)(.*)/m", '$1$1$2', $textvar); This pattern/function is over-performing. It doesn't make any sense to pick up the second capture group, just to put it back down unaltered. In fact, there is no need for capture groups or character classes for this task. I will also recommend that you use a consistent pattern delimiter for all patterns in your script. A tilde is a good/popular choice because it shouldn't interfere with any of your patterns and will help to prevent any accidental typos and escaping problems elsewhere. To double the leading whitespace on each line, just use this (there's no use doubling zero whitespaces, so match one-or-more with +):

    preg_replace('~^ +~m', '$0$0', $textvar);
    
  6. $textvarArr = preg_split("/\r\n|\n|\r/", $textvar) is more succinctly written as:

    $textvarArr = preg_split("~\R~", $textvar)
    
  7. Whenever I see preg_match() called inside of a loop so that replacement actions can be performed, this is a classic indicator that preg_replace_callback() should be called.

    $textvarArr = preg_split("~\R~", $textvar);
    foreach ($textvarArr as $key => $value) {
        preg_match('~=>\\s(.*?),~', $value, $newvalue);
        if (!empty($newvalue)) {
            $newvalue[1]      = str_replace("'", "", $newvalue[1]);
            $typeval          = myGetType($newvalue[1]);
            $value            = str_replace("=> ", "=> " . $typeval . ': ', $value);
            $textvarArr[$key] = $value;
        }
    }
    

    can become:

    $textvar = preg_replace_callback(
        "~ => \K\V+(?=,)~", 
        function ($m) {
            return myGetType(str_replace("'", "", $m[0])) . ": {$m[0]}";
        },
        $textvar
    );
    // then proceed with something like...
    $textvarArr = preg_replace(["/\s*array\s\($/", "/\)(,)?$/", "/\s=>\s$/"], [NULL, ']$1', ' => array ['], preg_split("~\R~", $textvar));
    
  8. I've got to say that this looks a little dangerous...

    if (substr($textvar, -1) == '[') {
        $textvar = str_replace("[", "[]", $textvar);
    }
    

    I mean, you are checking the final character, then doing a full text scan and replace operation. This deserves a bit of double-checking. Don't you only mean to close the final square brace? Maybe...

    if (substr($textvar, -1) == '[') {
        $textvar .= "]";
    }
    

I didn't get a chance to properly review (but it could probably use a little polish/simplifying):

$textvarArr = preg_replace(["/\s*array\s\($/", "/\)(,)?$/", "/\s=>\s$/"], [NULL, ']$1', ' => array ['], preg_split("~\R~", $textvar));
$textvar    = join(PHP_EOL, array_filter(["array ["] + $textvarArr));

I ran our outputs on a diffchecker program to be sure that my script returns the exact same output as yours.

Overall, an interesting exercise, good job.

\$\endgroup\$
  • 1
    \$\begingroup\$ I have no idea the ramifications of rebuilding this script on that function. I have only ever seen that function used on this site (never anywhere else in the SE network). I am happy for you to demonstrate. \$\endgroup\$ – mickmackusa Oct 5 at 13:35
  • 1
    \$\begingroup\$ I don't see resource on that list either. \$\endgroup\$ – mickmackusa Oct 5 at 13:41
  • \$\begingroup\$ I need to clarify that the main variable result of var_export is a string and therefore I cannot use is_string (), is_float, is_integer, because is_string will always be valid, I must evaluate if the string can be converted to the other types to validate them so use (int) (float) etc, and then I make the comparison == (without type) \$\endgroup\$ – walter nuñez Oct 5 at 15:43
  • 1
    \$\begingroup\$ i have make other post result of it... if you can help me with this: stackoverflow.com/questions/58458584/… \$\endgroup\$ – walter nuñez Oct 22 at 14:37
  • 1
    \$\begingroup\$ Gotta sleep. Maybe tomorrow. \$\endgroup\$ – mickmackusa Oct 22 at 14:42

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.