contributte / datagrid

:muscle: DataGrid for Nette Framework: filtering, sorting, pagination, tree view, table view, translator, etc
https://contributte.org/packages/contributte/datagrid/
MIT License
290 stars 194 forks source link

FilterableDataSource::filter getConditionCallback data_source reference #559

Closed mrceperka closed 7 years ago

mrceperka commented 7 years ago

Hi, I have data_source which is an array. I've used setCondition at filter component for custom filtering.


Now, because my data_source is array, it will not be passed as object would. Hotfix is easy, just add & before $this->data_source.

public function filter(array $filters)
{
    foreach ($filters as $filter) {
        if ($filter->isValueSet()) {
            if ($filter->hasConditionCallback()) {
                Callback::invokeArgs(
                    $filter->getConditionCallback(),
                      // ↓ CHANGE HERE
                    [& $this->data_source, $filter->getValue()]
                );
            } else {
                              //    ...
            }
        }
    }

    return $this;
}


Or, you could stop mutating the dara source with callback and instead require it to return new data_source...

mrceperka commented 7 years ago

Got pull request ready with this one line fix.

mrceperka commented 7 years ago

Bump. What do u think @paveljanda ?

paveljanda commented 7 years ago

@mrceperka Why don't you just return the filtered array in you condition callback?

mrceperka commented 7 years ago

How would that help? It is all based on modifying existing data_source. I do not see any

$this->data_source = Callback::invokeArgs(...)
paveljanda commented 7 years ago

Right. Do this than:

Filter::setCondition(function(& $data, $value) { ... });
paveljanda commented 7 years ago

I should mention that in docu. Hmmm

mrceperka commented 7 years ago

that will probably yell at me, that variable that i am expecting is not passed via refference...

paveljanda commented 7 years ago

It doesn't work in PHP like that (It's not actually a reference to some memory block)

<?php

function fooMe(& $foo)
{
    $foo = 'foo';
}

$a = 'a';

fooMe($a);

echo $a; // foo
mrceperka commented 7 years ago

Fine, I've propably did not try this little hack of yours. Thank you.

paveljanda commented 7 years ago

Well it's not a hack. :D

It's just that I am trying to make it work similar way as the fluent interface of Dibi\Fluent and other objects that change themselves with every method called above them.

mrceperka commented 7 years ago

Yes, I've seen those 'self mutating' collections. Better approach is returning new collection. But this is propably better (or easier?) solution for compatibility. Or you can CLONE them all :D