CODEWITHSUNDEEP
php
Cyclone
Cyclone

Reputation: 18295

Why does this loop give up halfway through?

$list is a very large array with a series of words, some of which are greater than 5 characters and some of which are smaller. This loop will unset all values where the length of the word is longer than 5 chars.

for($i = 0; $i < count($list); $i++)
{
    $list[$i] = trim($list[$i]);
    if(strlen($list[$i]) > 5)
    {
        unset($list[$i]);
    }
}

However, it does that for about half of $list and cuts out dead, there's words which are significantly longer than 5 letters still left in the list, but only starting approximately halfway through. What makes it quit? Should I run that loop multiple times?

Upvotes: 2

Views: 228

Answers (8)

Brad Koch
Brad Koch

Reputation: 20337

The count() is the problem. As a solution, I'm partial to foreach loops.

foreach ($list as $k => $v) {
    $list[$k] = trim($v);

    if (strlen($list[$k]) < 5) {
        unset($list[$k]);
    }
}

I think they offer a bit better clarity than the array_ functions.

Upvotes: 1

DaveRandom
DaveRandom

Reputation: 88697

As you unset() elements, count() decreases. This means that what you have describe is very likely to happen. You would be better calling count() once before the loop, storing it in a variable, and evaluating that at the top of the loop.

Upvotes: 0

zzzzBov
zzzzBov

Reputation: 179256

It fails because you're incrementing i while shrinking the total length of the array, which means at some point i is longer than the current array length.

A safer way of doing this is to count backwards:

for ( $i = count($list); $i--; )
{
    $list[$i] = trim($list[$i]);
    if(strlen($list[$i]) > 5)
    {
        unset($list[$i]);
    }
}

Upvotes: 5

imbrizi
imbrizi

Reputation: 3838

Every time unset() is called the length of the array changes. Just store it in a variable before the loop:

$count = count($list);
for($i = 0; $i < $count; $i++)
{
   // your code
}

Upvotes: 0

Facundo Farias
Facundo Farias

Reputation: 408

Works amazingly fast in my netbook's poor hardware with a 10000 list of words from www.lipsum.com

function max_len_filter($var) {
    if(strlen($var) > 5) return false;
    return true;
}
$words = array_filter($words, 'max_len_filter');
var_dump($words);

Hope it works for you

Upvotes: 0

mfonda
mfonda

Reputation: 8003

You're calling count every time, and you're unsetting things during the loop. Call count only once, before the loop. This is good practice to do anyways, as it greatly reduces the number of function calls you have to make.

$count = count($list);
for($i = 0; $i < $count; $i++)
{
    $list[$i] = trim($list[$i]);
    if(strlen($list[$i]) > 5)
    {
        unset($list[$i]);
    }
}

Or just for fun, use array_map/array_filter:

$list = array_map('trim', $list);
$list = array_filter($list, function($s) { return strlen($s) <= 5; });

Upvotes: 7

Boris Nikolaevich
Boris Nikolaevich

Reputation: 1461

You need to loop through from count($list) down to 0; when you remove starting at the beginning, you actually have fewer items by the time you get to the end.

It would also be useful to store count($list) before the loop instead of evaluating it each time.

Upvotes: 0

CaNNaDaRk
CaNNaDaRk

Reputation: 1322

Count($list) is changing as you unset $list elements so every iteration count returns a smaller value.

Upvotes: 2

Related Questions