6
\$\begingroup\$

This is a followup of this question.

@Corbin made a very interesting review, and I re-wrote most of the code.

function hex_dump( $value )
{

    $to_hex = function( $number ){
        $hex = strtoupper( dechex( $number ) );
        //if we don't check if the number is 0, it won't fill the whole space
        return ( $number === 0 || strlen( $hex ) & 1 ? '0' : '' ) . $hex;
    };
    $lines = array();
    $start_time = microtime(true);

    switch( gettype( $value ) )
    {
        case 'string':

            foreach( str_split( $value, 16 ) as $k => $line )
            {
                $lines[$k] = array();

                for( $i = 0, $l = strlen($line); $i<$l; $i++)
                {
                    $lines[$k][$i] = $to_hex( ord( $line[$i] ) );
                }
            }

            break;

        case 'double':
        case 'integer':
        case 'boolean':
        case 'NULL':
            $lines[0] = str_split( $to_hex( $value ), 2 );
            break;

        case 'array':
            foreach( array_chunk( $value, 16, false ) as $k => $chunk )
            {
                foreach( $chunk as $k => $item )
                {
                    switch( gettype( $item ) )
                    {
                        case 'double':
                        case 'integer':
                        case 'boolean':
                        case 'NULL':
                            if( $item > 255 )
                            {
                                trigger_error( 'hex_dump() numbers in a byte array cannot be higher than 255 on index ' . $k, E_USER_WARNING );
                            }
                            //we need to fix the number, if it isn't a single byte
                            $chunk[$k] = $to_hex( $item & 255 );
                            break;
                        case 'string':
                            if( strlen( $item ) > 1 )
                            {
                                trigger_error( 'hex_dump() strings in a byte array cannot have more than 1 character on index ' . $k, E_USER_WARNING );
                            }
                            //disregard the remaining of the string, since only the 1st char matters
                            $chunk[$k] = $to_hex( ord( $item[0] ) );
                            break;
                        default:
                            $chunk[$k] = '--';
                            trigger_error( 'hex_dump() invalid value on index ' . $k, E_USER_WARNING );
                    }
                }   
                $lines[] = $chunk;
            }
            break;

        default:
            trigger_error( 'Invalid value type passed', E_USER_WARNING );
            return false;
    }

    $line_count = count( $lines );

    $num_length = strlen( dechex( $line_count ) ) + 1;
    $num_length = $num_length + ( $num_length % 2 );

    $header = str_repeat( ' ',  $num_length ) . ' |';
    for( $number = 0; $number < 16; $number++ )
    {
        $header .= '0' . dechex( $number ) . '|';
    }
    $header .= '      TEXT      ';

    echo $header, PHP_EOL;

    $separator = str_repeat( '-', strlen( $header ) );

    foreach( $lines as $current_line => &$line )
    {
        echo $separator, PHP_EOL;

        //the number must be padded with 0s in the beginning, to the size of the highest line number
        echo str_pad( strtoupper( dechex( $current_line ) ), $num_length - 1, '0', STR_PAD_LEFT ),'0 |';

        //outputs what is in the line, regardless of the length
        echo implode( '|', $line ), '|';

        //we need to fix the missing spaces in the output
        $missing = 16 - count( $line );
        if( $missing > 0 )
        {
            do
            {
                echo '  |';
            }
            while( --$missing );
        }

        foreach( $line as $value )
        {
            if( $value == '--' )
            {
                // replacement character, for invalid values on byte arrays
                echo "\xEF\xBF\xBD";
            }
            else
            {
                $value =  hexdec( $value );

                echo $value < 32 || $value > 126 ? '.' : chr( $value );
            }
        }

        echo PHP_EOL;
    }

    $stats = array(
        'lines' => $line_count,
        //if there isn't a check to see if we have any line, this will cause errors
        'bytes' => $line_count ? ( $line_count * 16 ) - ( 16 - count( $lines[ $line_count - 1 ] ) ) : 0,
        'time' => microtime(true) - $start_time
    );

    echo str_repeat( '=', strlen( $separator ) ), PHP_EOL;
    echo str_pad( 'Lines: ' . $stats['lines'], 15, ' '), '| ';
    echo str_pad( 'Bytes: ' . $stats['bytes'], 16, ' '), '| ';
    echo 'Time: ', $stats['time'], 'ms', PHP_EOL, PHP_EOL;

    return $stats;
}

Something I forgot to mention on my previous question was the handling of invalid types.

Consider the following example:

hex_dump(array(array(),'too big',12345));

The following code will output something similar to:

<br />
<b>Warning</b>:  hex_dump() invalid value on index 0 in <b>/code/3kGyEY</b> on line <b>65</b><br />
<br />
<b>Warning</b>:  hex_dump() strings in a byte array cannot have more than 1 character on index 1 in <b>/code/3kGyEY</b> on line <b>58</b><br />
<br />
<b>Warning</b>:  hex_dump() numbers in a byte array cannot be higher than 255 on index 2 in <b>/code/3kGyEY</b> on line <b>50</b><br />
   |00|01|02|03|04|05|06|07|08|09|0a|0b|0c|0d|0e|0f|      TEXT      
--------------------------------------------------------------------
00 |--|74|39|  |  |  |  |  |  |  |  |  |  |  |  |  |�t9
====================================================================
Lines: 1       | Bytes: 3        | Time: 0.00012302398681641ms

@Corbin recommends that I handle arrays like how I handle if you run with an invalid value, but that would leave a ton of good data left behind.

List of actions taken, based on @Corbin's review:

  • I've created the closure $to_hex, which handles almost all the hexadecimal conversions and padding.
  • Tried to DRY my code as much as possible, but kindly ignored his suggestion to call it recursively.

That wasn't the goal: The goal is to receive an array of bytes, not an array of arbitrary values. - Regarding performance, I've replaced all the array_walk calls with foreach loops.

Also, I've tried to remove as many calls to str_split as possible. - I've separated the multiple echoes and it is a lot clearer now. - Even though his last advice was really good, the goal isn't to return the hex dump.

The idea is to use it like how we use var_dump.

With those actions taken and changes made, now it's time to ask:

  • What else can I improve?
  • Is there any other performance killers?
  • Is my code DRY now?
  • Is it more easily readable?
\$\endgroup\$
3
  • \$\begingroup\$ Will hex_dump(array(array(),'too big',12345)) output a bunch of warnings today, or did it just used to do so before? \$\endgroup\$ Jun 12, 2015 at 17:21
  • \$\begingroup\$ @SimonAndréForsberg If you pass an array, it is expected to be an array of bytes. None of those is 1 byte long. You can read the 2nd bullet point in the list. \$\endgroup\$ Jun 12, 2015 at 17:25
  • \$\begingroup\$ @SimonAndréForsberg Oh, sorry. If you mean to ask if it throws the warnings as the code is, yes it will. That is the goal. \$\endgroup\$ Jun 12, 2015 at 17:37

1 Answer 1

4
\$\begingroup\$

I've promised you a million times that I'd review this, and I am finally!

  • You've got extraneous space in your brackets all throughout your code:
        foreach( array_chunk( $value, 16, false ) as $k => $chunk )
        {
            foreach( $chunk as $k => $item )
            {
                switch( gettype( $item ) )
  • In your first switch I would recommend putting the smaller blocks first, as I always found it easier to read that way, this is my opiniontm, so keep that in mind.

  • You could consider putting all the cases on one line, like so:

    case 'double':
    case 'integer':
    case 'boolean':
    case 'NULL':
    case 'double': case 'integer': case 'boolean': case 'NULL':
  • The reasoning behind lines like: $header .= ' TEXT ' is a bit confusing, consider commenting some of your code with your reasoning.

  • There's some extraneous space in some of your lines, like in $value = hexdec( $value );

  • $num_length = $num_length + ( $num_length % 2 ): there's a += operator in PHP.

What I would suggest is to put your code through a formatter like: PHP Formatter.

\$\endgroup\$

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.