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

Remove query, filter, facet, transport and type class suffixes #303

Merged
merged 6 commits into from
Jan 16, 2013

Conversation

munkie
Copy link
Contributor

@munkie munkie commented Jan 16, 2013

Removed Query, Filter, Facet, Transport and Type suffixes form class names to avoid duplicating this part in namespace and class.

Abstract classes and Exceptions were left as it is. I checked ZF2 and Symfony2 naming convensions and both of them suggest naming abstract classes with Abstract prefix and exception classes with Exception suffix.

Thou some class names were interfering with php reserved keywords, so i had to rename them in different manner,

  • And and Or filters were renamed to BoolAnd and BoolOr, also renamed Not filter to BoolNot for consistency thou Not is not a reserved keyword.
  • Array query renamed to Simple query.

@@ -10,7 +11,7 @@
* @package Elastica
* @author James Boehmer <[email protected]>
*/
class NullTransport extends AbstractTransport
class Null extends AbstractTransport
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have expected, that this is also a reserved keyword.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Null is not a reserved keyword, here is full list http://php.net/manual/en/reserved.keywords.php.

ruflin added a commit that referenced this pull request Jan 16, 2013
Remove query, filter, facet, transport and type class suffixes
@ruflin ruflin merged commit c39ec1d into ruflin:master Jan 16, 2013
@ruflin
Copy link
Owner

ruflin commented Jan 16, 2013

Should it be BoolAnd or AndBool?

@munkie munkie deleted the remove-class-suffixes branch January 16, 2013 13:07
@munkie
Copy link
Contributor Author

munkie commented Jan 16, 2013

You think there is a point in renaming those classes to AndBool, OrBool and NotBool ?

@ruflin
Copy link
Owner

ruflin commented Jan 16, 2013

That is exactly what I'm not sure about. Which one is better and makes more sense. Why I thought it should perhaps be the other way around is because of autocompletion. People start to to Prefix for the prefix query and will start to type and for the Andquery. So autocomplete would work. On the other hand someone perhaps knows, he needs a bool query and starts to type.

@munkie
Copy link
Contributor Author

munkie commented Jan 16, 2013

I've tried PhpStorm autocomplete - it suggests BoolOr class when you type Or.

@ruflin
Copy link
Owner

ruflin commented Jan 16, 2013

Very helpful. At least that confirms again that PhpStorm is awesome :-)

I personally prefer the OrBool option, but as I described above I'm undecided.

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

Successfully merging this pull request may close these issues.

2 participants