Suggestion: support multiple filters of the same type

[fredemmott]

I like this way of combining filters: https://github.com/hhvm/user-documentation/blob/de3652909872a2017b7037fb56d9c7d3ab71c71d/bin/build-yaml-from-hhvm.php#L49-L50

[kilahm]

When adding multiple filters, in which way are they combined?

I.e., is

$builder->includeClasses()
->filterClasses($c ==> $c->getName() === 'SomeName')
->filterClasses($c ==> $c->getName() === 'OtherName')

equivalent to

$builder->includeClasses()
  ->filterClasses($c ==>
    $c->getName() === 'SomeName' ||
    $c->getName() === 'OtherName'
  )

or

$builder->includeClasses()
  ->filterClasses($c ==>
    $c->getName() === 'SomeName' &&
    $c->getName() === 'OtherName'
  )

In theory I agree that being able to write really short filters and explicitly list each one individually would be nice. I think the user would have to decide what that combination means though. As it stands, simply adding two filters in series is fairly ambiguous. I think I might rewrite your example as

(new ScannedDefinitionsYAMLBuilder($source, $parser, $destination))
  ->addFilter($x ==> 
    ScannedDefinitionFilters::IsHHSpecific($x)) ||
    !ScannedDefinitionFilters::ShouldNotDocument($x)
  )
  ->build();

This way the user is forced to think about how the filters are composed into more complex filters.

[kilahm]

I originally started writing this library with that type of API (as you may recall from my email). But being able to handle both ways of composing multiple filters makes things a bit confusing.

I mostly built this type of composition flexibility in a DB abstraction layer (that I haven't published). It looks something like:

$db->fetch(CollectionName::SomeTable)->where(
  $model->field->is('blah')
  ->orWhere($model->otherField->lt(15))
  ->asWellAs($model->thirdField->in(Vector{'one', 'two'}))
);

That would render to and SQL statement something like

SELECT * FROM `SomeTable` WHERE ( `field` = "blah" OR `otherField` < 15 ) AND ( `thirdField` IN ("one", "two") )

Note that this is not allowed

$db->fetch(CollectionName::SomeTable)->where(
  $model->field->is('blah')
  ->orWhere($model->otherField->lt(15))
  ->andWhere($model->thirdField->in(Vector{'one', 'two'}))
);

because the grouping is ambiguous.

I really wanted a fluent api to allow arbitrary composition of logical AND and OR statements. It gets pretty hairy, but it can be done.

[fredemmott]

I'm not aware of an API where adding a filter can increase the amount of data returned. && 'feels' common and matches expectations when using collecitions, eg $map->filter($a)->filter($b) == $map->filter($x ==> $a($x) && $b($x))

[kilahm]

Except then the only way you could compose the filters with an OR is to explicitly combine them in another function. That feels asymmetric to me.

The use case would be: "I would like classes in namespace A and I would like classes in namespace B and I would not like classes in namespace A\Test". I could see attempting to translate that sentence into a chain of filter calls on the builder very similar, if not identical to, the chain you mention.

[fredemmott]

IMO that doesn't match the meaning of the word 'filter'. if there were an 'include()' function I'd expect it to be an OR not an AND

[kilahm]

So something like this?

$builder
->includeClassesFilteredBy($filterA)
->includeClassesFilteredBy($filterB)
->filterAllClasses($filterC)
->filterAllClasses($filderD)

[fredemmott]

I'd be fine with that; though here's an alternative this would completely solve the 'filter' naming issue:

->onlyClassesMatching($filterC)
->onlyClassesMatching($filterD)

As a side note, implementation shortcut: https://github.com/fredemmott/definitions-finder/blob/master/src/FileParser.php#L115-L122

[kilahm]

I think the word "match" is a little to narrow for generic filters. I also agree with your point about Map::filter being a good example of the usage of the word "filter".

I would be OK having multiple filter calls always remove from the list now that we agree on having multiple include calls always adding to the list. Before, having only a single include and multiple filters, it was difficult to express "I want classes in namespace A and classes in namespace B" by chaining simple filters.

I could also see having methods like includeAllClasses() which would be equivalent to includeClasses($c ==> true). Similarly, have filterAllClasses() equivalent to filterClasses($c ==> false).