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 question is the follow up of the question:

UIntArray class in PHP

@Brythan provided a really nice review and I have improved my class.

I have addressed almost all the issues he pointed out (the only one I didn't do anything was about the UIntArray::pop() function, that was intentional) and still made some improvements on my own.

final class UintArray implements \Countable,\ArrayAccess,\IteratorAggregate
{
    private $data = array();   //all the saved data will be here
    private $maximum_length = 0;  //maximum length of the array
    //current count, can't use $count because accessing $this->count might give troubles
    private $current_count = 0; 

    //numbers of bytes to store
    const UIntDefault = 0; //default value
    const UInt8 = 1;
    const UInt16 = 2;
    const UInt24 = 3;
    const UInt32 = 4;

    //the index is kept here only for readability.
    private static $bit_masks=array(
        0 => 0xFFFFFFFE, //default bit mask
        1 => 0xFF,
        2 => 0xFFFF,
        3 => 0xFFFFFF,
        //0xFFFFFFFF is -1 on 32bit systems, but 4294967295 on 64 bits
        4 => 0xFFFFFFFE
    );

    //used to be sure the value doesn't go above the maximum value
    private $bit_mask;

    private static function sanitize($value, $bit_mask = 0)
    {
        //sanitize the value, to ensure it is less or equal to the desired mask
        return $value & ( $bit_mask ? $bit_mask : self::$bit_masks[self::UInt32]);
    }

    public function __construct($maximum_length, $bytes_per_element = 0)
    {
        //set the length to a 32bit integer
        $maximum_length = self::sanitize($maximum_length);

        //stores the maximum length, check if it higher than 0
        $this->maximum_length = ( $maximum_length > 0 ) ? $maximum_length : 1;

        //sets the bit mask to be used
        $this->bit_mask = self::$bit_masks[ ( $bytes_per_element >= 1 && $bytes_per_element <= 4 ) ? $bytes_per_element : self::UIntDefault];

        //fill the array ahead, so it's space will be all reserved
        //in theory, this will be faster than creating elements each time
        $this->data = array_fill(0, $maximum_length, 0);
    }

    //countable
    public function count(){return $this->current_count;}

    //arrayaccess
    public function offsetSet($offset, $value)
    {
        $this->__set($offset, $value);
    }
    //used with isset($arr[<offset>]);
    public function offsetExists($offset)
    {
        $offset = self::sanitize($offset);

        //if the offset is within the limits
        if($offset > 0 && $offset <= $this->maximum_length)
        {
            return isset($this->data[$offset]);
        }
        return false;
    }
    //used with unset($arr[<offset>]);
    public function offsetUnset($offset)
    {
        $offset = self::sanitize($offset);

        //if the offset is withing the limits
        if($offset > 0 && $offset <= $this->maximum_length)
        {
            $this->data[$offset]=0;

            if($offset == $this->current_count-1)
            {
                //if we are unsetting the last element, we can safely reduce the count
                --$this->current_count;
            }
        }
    }
    //used with $arr[<offset>];
    public function offsetGet($offset)
    {
        return $this->__get($offset);
    }

    //iteratoraggregate
    //used on the foreach loop
    public function getIterator(){return new ArrayIterator($this->toArray());}

    //magic methods
    public function __toString()
    {
        //replicated the default behavior of converting an array to string
        if(error_reporting() & E_NOTICE)
        {
            @trigger_error('Array to string conversion', E_USER_NOTICE);
        }
        return 'Array';
    }
    public function __invoke(){return $a=&$this->data;}
    public function __set_state(){return $a=&$this->data;}
    public function __set($offset, $value)
    {
        //allows to set $arr[]=<value>;
        if(is_null($offset))
        {
            //verifies if the array is full. returns false if it is.
            if($this->current_count >= $this->maximum_length)
            {
                return false;
            }

            //provides the offset to set the value
            $offset = $this->current_count++;
        }
        //verifies if the $offset is within the allowed limits
        else if( $offset < 0 || $offset > $this->maximum_length)
        {
            return false;
        }

        $this->data[ self::sanitize($offset) ] = self::sanitize($value, $this->bit_mask);
    }
    public function __get($offset)
    {
        $offset = self::sanitize($offset);

        //returns a dummy variable, just in case someone uses the increment (++) or decrement (--) operators
        $dummy = isset($this->data[$offset]) ? $this->data[$offset] : null;
        return $dummy;
    }
    public function __sleep(){return $this->data;}

    //other functionality methods
    public function push()
    {   
        //retrieve all the arguments, saving one variable
        foreach(func_get_args() as $value)
        {
            //if the array still have space
            if( $this->current_count < $this->maximum_length )
            {
                //add to the array, increasing the count
                $this->data[ $this->current_count++ ] = self::sanitize($value, $this->bit_mask);
            }
            //if the array is full, exit the loop
            else break;
        }

        //returns the number of elements
        //this replicated the behaviour of the function array_push()
        //Documentation: http://php.net/manual/en/function.array-push.php
        //Test-case (using array_push()): http://ideone.com/PrTo8m
        return $this->current_count;
    }

    public function pop()
    {
        //if the array is empty
        if($this->current_count < 1)
        {
            return null;
        }
        else
        {
            //decreases the count and stores the last value
            $value = $this->data[ --$this->current_count ];

            //stores 0 on the last value
            $this->data[ $this->current_count ]=0;

            //returns the last element
            return $value;
        }
    }

    public function maxlen(){return $this->maximum_length;}
    public function bitmask(){return $this->bit_mask;}
    public function toArray(){return array_slice($this->data, 0, $this->current_count);}
}

In terms of functionality and usage, it works exactly the same.

The only change was the function UIntArray::bits() was renamed to UIntArray::bitmask(), to address the name change.

Is there anything, both in coding style and performance, that I should address and fix?

share|improve this question
    
why check if(error_reporting() & E_NOTICE) in your __toString method, why not simply return (string) []; (return a array to string cast), so you get the same behavior, with a lot less code –  Elias Van Ootegem Nov 19 '14 at 9:25
    
@EliasVanOotegem That is a good one, but less obvious. I would need to add (another) comment. And I wouldn't use [] in a PHP context (not yet, at least while PHP 5.3 is around). –  Ismael Miguel Nov 19 '14 at 9:34
    
PHP 5.3 is no longer supported, officially, and the short array syntax is available in 5.4 and up, so all of the supported versions can handle it. Also: comments are fine, your code posted here contains, if anything, too little comments –  Elias Van Ootegem Nov 19 '14 at 9:51
    
@EliasVanOotegem Sadly, I'm still trapped with PHP 5.3 :/ And to be honest: I prefer this way to create an array. Using array() for PHP and [] for Javascript. I've always been used to that too. –  Ismael Miguel Nov 19 '14 at 9:57

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.