private $arr=array(); //all the saved data will be here
private $maxlen=0; //maximum length of the array
private $cnt=0; //current count
Rather than use comments that only appear in one place, it is generally better to use descriptive names:
private $saved_data = array();
private $maximum_length = 0;
private $current_count = 0;
$count would probably be enough, as "saved" and "current" are implied.
I also added whitespace between tokens. The compiler won't care, but this often makes it easier for humans to read.
I'd call this a
$bit_mask. Also, as a general rule, I prefer to only name collections in the plural. This is a scalar, so I would try to name it in the singular.
I wouldn't use a comment here, as this isn't where it's used.
public function __construct($maxlen,$b=4)
I'd write this
public function __construct($maximum_length, $bytes_per_element = 4)
That makes it clearer why you would pass those parameters.
//stores the maximum length, check if it higher than 0
As before, I'd rewrite this
$this->maximum_length = ($maximum_length >> 0) > 0 ? $maximum_length >> 0 : 1;
I removed your comment, as that just restates what I can read from the code. I'd still like a comment here though. It's unclear to me why you are shifting the bits right by zero places. The only reason that I can see is that it forces PHP to treat
$maximum_length as an integer. A comment would be helpful, but I wouldn't put it here. Instead use a helper function here.
$this->maximum_length = UIntArray::make_int32($maximum_length > 0 ? $maximum_length : 1);
which says what it is doing explicitly rather than relying on a side effect. Also, I put it all the way around, as you want to force it to be a 32 bit integer all the time, not just when the value is not the default.
private static function make_int32($value)
// Doing a shift forces PHP to treat the value as a 32 bit integer.
// Shifting by 0 means that this is otherwise a no-op.
return $value >> 0;
Now there may be a reason why the no-op right shift is better than an explicit cast, but if so, tell me. Also, this would be a great time to define a unit test that shows me what can go wrong. That way if someone optimizes the right shift out of the code, then at least it will fail unit testing immediately.
This is more commonly written
which makes it clearer that you are directing multiple cases to the same result.
//fill the array ahead, so it's space will be all reserved
// fill the array ahead, so its space will be all reserved
Should use the possessive "its" there rather than the contraction "it's".
//verifies if the offset is valid, and if still have space
So if the array is full, we can no longer set any elements? That seems like a bug. We should not be able to add elements in that case, but we should still be able to change existing elements.
if ( 0 > $offset || ( UIntArray::make_int32($offset) >= $this->maximum_length ) )
is not correct.
offsetSet does not return a value, so this won't work. It's not clear what you should do instead. Throwing an exception is one possibility.
//allows for $arr=<value>;
Ok, now I see why you wanted to check that the array wasn't full, but I still think that you shouldn't have done it where you did. Instead
// $uint_array = <value>; will set the $offset to null
if ( is_null($offset) )
if ( $this->count >= $this->maximum_length )
// panic somehow and do not proceed
$offset = $this->count++;
$this->data[UIntArray::make_int32($offset)] = $this->sanitize($value);
This only checks
maximum_length when you would be increasing the
null, we set
$offset to equal
count (which will be one greater than the index of the last element) and increment count. Your original version had a bug where it incremented the count first, which actually sets the offset just beyond the end of the array.
Doing things this way gets rid of the
else and reduces repeated code. We do the same thing for either case. We just have to prime the
$offset if it wasn't explicitly passed.
Note that this also requires a helper function:
private function sanitize($value)
// only return the bits possible for this size of element
return $value & $this->bits;
You're doing this a lot, so should define it in one place.
public function offsetGet($offset)
I don't think that will work. The
offsetGet function returns by reference, but
null is a value. I think that you have to do something like
: $dummy = null;
so that it has a variable to reference.
This also brings up another problem. The
offsetGet function will return a reference to the underlying data structure which can then be used in assignments. If the caller does this, they can bypass the bit mask that you use in assignments. For that matter, the caller could store a non-integer in the array. So maybe what you want to do is
$offset = UIntArray::make_int32($offset);
$dummy = isset( $this->data[$offset])
That always returns a dummy reference. Of course, if you do that, you may block some assignments that you would want to allow. Perhaps there is some way to return a special
$dummy that will trigger the proper behavior on assignment or update.
foreach($args as &$value)
//if we still have space
//add to the array, increasing the count
//if not, return the size (replicates array_push behaviour)
else return $this->cnt;
//returns the size (replicates array_push behaviour)
I'd write this
foreach ( $args as &$value )
if ( $this->count < $this->maximum_length )
$this->data[$this->count++] = $this->sanitize($value);
// when out of space, stop processing arguments
// returns the number of elements in the array (replicates array_push behaviour)
I got rid of the comments that just said the same thing as the code. I left the comment that explained why we were doing this uncommon thing.
I changed the
else because you were doing the same thing in the
else as you would if it just fell out of the loop. Doing it this way means that we only have one
return statement to maintain.
//if the array is empty
This seems more easily written as
if ( $this->count <= 0 )
which shouldn't require a comment. Also, this covers more potential issues, as the previous check only made sure that the count wasn't 0. This covers negative offsets as well.
//decreases the count and stores the last value
//stores 0 on the last value
//returns the last element
This seems overly complicated. Under what circumstance does the value at a particular offset outside the array matter?
This covers the basic functionality and is easier to read.