deltacloud-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From David Lutterkort <lut...@redhat.com>
Subject Re: [PATCH core 4/8] Core: Added possibility to run filter_on with multiple attrs
Date Tue, 19 Feb 2013 00:36:42 GMT
On Mon, 2013-02-18 at 14:41 +0100, mfojtik@redhat.com wrote:
> From: Michal Fojtik <mfojtik@redhat.com>
> 
> So instead of writing:
> 
> instances = filter_on instances, :id, opts
> instances = filter_on instances, :realm_id, opts
> instances = filter_on instances, :state, opts
> 
> You can now use:
> 
> instances = filter_on instances, opts, :id, :realm_id, :state
> 
> The backward compatibility with old filter_on included.
> 
> Signed-off-by: Michal fojtik <mfojtik@redhat.com>

NAK. I like the idea, but it has a couple flaws:

      * instead of doing gymnastics to support the old and the new
        filter_on gymnastics, why not introduce a new filter method that
        cab do something like filter(coll, :id => [42, 43], :name =>
        ['A', 'B'], :state => "RUNNING") IOW, use the keys of the
        passed-in hash to determine which fields to filter on ?
        filter_on could then just be a simple frontend to the new filter
        method
      * your implementation now modifies the collection that is passed
        in, whereas the old one returned a copy; while that may not
        matter, it's risky

David



Mime
View raw message