No, there is nothing wrong with doing it like that.

I guess it depends on collection size if it is big, there may be performance issues for sure :)

How big, and how come?

did you read http://laravel.com/docs/eloquent#collections regarding filter() and each()

It seems like doing it that way is kind of fuzzy and procedural. Instead, you could define a custom collection that would automatically do the work for you. For example:

class Collection extends Illuminate\Support\Collection
{
    public function cancelled()
    {
        return $this->filter(function($order) {
            return $order->isCancelled();
        }
    }
}

And now, you can get your cancelled orders just by doing this:

$orders = Order::somethingthatresultsinacollectionobject();
foreach($orders->cancelled() as $order) {
    //do whatever
}

See: http://laravel.com/docs/eloquent#collections