0
\$\begingroup\$

I'm trying to remove the first $offset elements from an array and reduce the offset for future adds to the array. The loop must also end if I reach the end of the array.

$max1 = $offset;
$max2 = count($elements);
for ($i=0; ($i<$max1 && $i<$max2); $i++) {
    unset($elements[$i]);
    $offset--;
}

I'm trying to find a better way to do it. At least a way to use the values of $offset and count($elements) (which are changing) without having to create aux variables.

\$\endgroup\$
3
  • 4
    \$\begingroup\$ What task does this code accomplish? Please tell us, and also make that the title of the question via edit. Maybe you missed the placeholder on the title element: "State the task that your code accomplishes. Make your title distinctive.". Also from How to Ask: "State what your code does in your title, not your main concerns about it.". \$\endgroup\$ Commented May 11, 2018 at 15:19
  • \$\begingroup\$ What @SamOnela said. You were given the exact same advice when you posted this question on StackOverflow. \$\endgroup\$
    – Dave
    Commented May 11, 2018 at 15:56
  • \$\begingroup\$ I'm trying to be as much specific as I can. I changed the title but I think the task is clear, I don't really know what could be unclear in the description. \$\endgroup\$
    – CarlosAS
    Commented May 12, 2018 at 7:52

1 Answer 1

3
\$\begingroup\$

Code style

Variable names

Make sure that your variable names are descriptive of what they contain. In particular $max1 and $max2 use a poor naming style as it is unclear what you expect them to contain.

Whitespace

Make sure to keep whitespace around your operators, even in a for-loop. This increases readability, and thus decreases the chance for bugs.

Unneeded bookkeeping

You currently use $offset--; but do not use the result. Remove code you do not use.

Bugs

Your current code only works once, because you keep the keys. Considering the following code:

$offset = 3;
$elements = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13];

$max1 = $offset;
$max2 = count($elements);
for ($i=0; ($i<$max1 && $i<$max2); $i++) {
    unset($elements[$i]);
    $offset--;
}

$max1 = $offset;
$max2 = count($elements);
for ($i=0; ($i<$max1 && $i<$max2); $i++) {
    unset($elements[$i]);
    $offset--;
}

var_dump($elements);

// Result: array(11) { [3]=> int(3) [4]=> int(4) [5]=> int(5) [6]=> int(6) [7]=> int(7) [8]=> int(8) [9]=> int(9) [10]=> int(10) [11]=> int(11) [12]=> int(12) [13]=> int(13) }

You would expect it to remove the first 3 elements twice, but because the keys are not reset, the second iteration of your code will try to remove key 0, 1 and 2, while the first key is 3.

Algorithm

Your current code modifies an array in-place while keeping the keys. Modifying arrays while looping over them is aweful in any language, as you need to be mindful that conditions in loops may be pre-calculated, and element pointers may skip elements.

Since you use a for-loop, I can assume you use an array with numeric keys, which makes it odd that you would want to keep the current keys.

Alternative creating a new array

In php you can use array_slice to obtain a new array with only a slice of the original array. This will reset the keys of the array.

$elements = array_slice($elements, $offset);

No loop required.

Alternative deleting elements

You can use array_splice to modify an array in-place by deleting a consecutive chunk of array elements. It returns the deleted elements and the original array has a few less elements. This will also reset the keys.

array_splice($elements, 0, $offset);

Again, no loop required.

Alternative with a loop

You can also use a loop and use array_shift to remove the first element of the array repeatedly. For numeric keys the keys are reset, while literal keys are left untouched.

for ($i = 0; $i < $offset; $i++) {
  $result = array_shift($elements);
  if (is_null($result)) {
    // The array is empty, so safe us some time
    break;
  }
}

Filter to keep keys

If we want to keep keys, we can use array_filter. The following code works exactly like your own code, as in: It works the first time you invoke it, and then it breaks.

$elements = array_filter(
  $elements,
  function ($item, $index) use ($offset) {
    return $index >= $offset;
  },
  ARRAY_FILTER_USE_BOTH
);

You can improve that code by using and checking against $offset + $first_key.

reset($elements);
$first_key = key($array);
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.