Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

This is a followup of the following question: PHP function to create a Hex dump

@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 @Cobin'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 the changes made, now it's time to ask:

  • What else can I improve?
  • Is there any other performance killer?
  • Is my code DRY now?
  • Is it more easily readable?
share|improve this question
    
Will hex_dump(array(array(),'too big',12345)) output a bunch of warnings today, or did it just used to do so before? –  Simon Forsberg Jun 12 at 17:21
    
@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. –  Ismael Miguel Jun 12 at 17:25
    
@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. –  Ismael Miguel Jun 12 at 17:37

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Browse other questions tagged or ask your own question.