# UIntArray class in PHP (part 2)

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?

• 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 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). 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 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. Nov 19 '14 at 9:57

Your foreach loop can be simplified by making the break a guard clause:

    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; }  Instead, do it like:  foreach(func_get_args() as$value)
{
//if the array is out of space
if( $this->current_count >=$this->maximum_length )
{
break;
}

//add to the array, increasing the count
$this->data[$this->current_count++ ] = self::sanitize($value,$this->bit_mask);
}


That makes the conditions simpler to follow (no else statement).

Similarly, when you have a return statement, there's no need for an else, this code:

    //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; }  could instead be:  //if the array is empty if($this->current_count < 1)
{
return null;
}
//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;


Finally, even for 1-liner if-statements, you should indent things correctly. This last block:

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);}


should be:

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);
}


Why is the class named UintArray, while the constants are named like UInt8? Even if PHP is case-insensitive, it would be nice for you to be consistent in capitalization.

UIntDefault = 0 seems weird to me. Why not define UIntDefault = UInt32, since that's what you mean? Why is the bitmask for UInt32 0xFFFFFFFE instead of 0xFFFFFFFF? That rounds your data to even numbers.

When defining $bit_masks, there is no obvious correspondence with the UIntn constants that were just defined. I suggest: private static$bit_masks = array(
UInt8  => 0xFF,
UInt16 => 0xFFFF,
…
);


Why do offsets need to be sanitize()d? You want to validate the offset, not sanitize it. I don't recommend trying to make sense of bad input, because garbage in = garbage out. Some languages interpret negative array indices as counting backwards from the end — maybe you want to do that? But it's not idiomatic for PHP, so I recommend either emulating the standard behaviour for PHP arrays or explicitly failing.

The way you keep track of $current_count is buggy. In a normal array, you should be able to do $a = array(); $a[0] = "first";$a[] = "second";. However, if you try that with your UintArray, the second entry will overwrite the first, because you only maintain a correct \$current_count if you stick to the push()/pop()/offsetUnset() interface.

__sleep() is supposed to serialize the array, but you only serialize the data, and discard the type.

• "Why is the bitmask for UInt32 0xFFFFFFFE instead of 0xFFFFFFFF?" -> That's addrfessed in the comment, just above. "Why do offsets need to be sanitize()d? Do you not trust the caller to give you offsets that are integers?" -> Never trust user input. Also, It is to make sure that the integer values match the bitmask. About the last point, it is a really good one. I did that on purpose. The idea is to really return an array. The bug you mentioned, I need to make some testing. Jun 19 '15 at 19:13
• Nice catch on the 0xFFFFFFFE bug. It should be 0xEFFFFFFF instead. Jun 19 '15 at 19:19
• One thing I've noticed is that you have to use self::UInt8 instead of only UInt8 in the array. Jun 19 '15 at 19:34
• How would sanitizing an invalid offset make it any better, though? Jun 19 '15 at 19:57
• Sanitizing would limit the offset to integer-only and to prevent that the offset it outside the boundaries. Also, I've fixed the mentioned bug. It really was a nice catch. Jun 19 '15 at 20:04