Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Aggregation/Query/Processor class namings standardisation #1824

Open
deguif opened this issue Oct 21, 2020 · 1 comment
Open

Aggregation/Query/Processor class namings standardisation #1824

deguif opened this issue Oct 21, 2020 · 1 comment

Comments

@deguif
Copy link
Collaborator

deguif commented Oct 21, 2020

Previously some class were required to be named with a suffix to avoid conflicting with some reserved keywords.
For examples these classes use different naming than others:

  • Elastica\Query\BoolQuery instead of Elastica\Query\Bool previously
  • Elastica\Query\MatchQuery instead of Elastica\Query\Match previously
  • Elastica\Aggregation\GlobalAggregation
  • Elastica\Aggregation\ParentAggregation
  • Elastica\Processor\ForeachProcessor in PR Add foreach processor #1813

I think it would be nice to standardise the suffix strategy by suffixing all classes from Aggregation, Query and Processor namespaces. It could be done in a BC manner by creating a new class with the suffix part and making the previous class extends from it (which would trigger a deprecation too to help fixing it).

Regarding _getBaseName()method each base abstract class like Elastica\Aggregration\AbstractAggregation, Elastica\Query\AbstractQuery, Elastica\Processor\AbstractProcessor could take care of removing the suffix by overriding the method.

What do you think?

@ruflin
Copy link
Owner

ruflin commented Oct 26, 2020

I'm not a big fan of having the directory name repeated also in the class name itself. My hope initially was that this would be the exception, but it seems we have more and more use cases around it.

Is there any specific issue that triggered this or more generally to unify things?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants