search

erichard / elasticsearch-query-builder

Build query for an ElasticSearch client using a fluent interface
MIT License
39 stars 20 forks source link

Added in constructors with arguments, setters, updated Query class to reflect this and added in a test case #11

Closed byt3sage closed 2 years ago

byt3sage commented 2 years ago

Relating to: #10 #7

I had to do this quickly as I'm currently using this package inside a project so needed to get it working ASAP. I've added in class constructors for all the query classes as well as initialising these arguments within the constructor. I have also added in the GeoShape query to this with relating test. Furthermore, all Query classes now have setters within them.

Any feedback is welcome.

Sidenote: We need some more tests for other query classes. I'll do that once this is ready and merged.

CC @erichard

erichard commented 2 years ago

constructors arguments cannot be nullable if they are needed to build the query

byt3sage commented 2 years ago

You're correct - I misread what you actually wanted, I thought that was a tad strange.

Just to confirm @erichard , you want all needed attributes for a query to be set via a constructor and optional attributes to be set via setters?

erichard commented 2 years ago

@jaetooledev Yes exactly. Sorry for not being clear :pray:

byt3sage commented 2 years ago

@erichard how about this? required arguments are set via constructors, optional arguments are set via setters.

If this is okay - might it be worth me removing the checks that non-optional fields are empty in some of the queries since with type hinting they can't be null

erichard commented 2 years ago

Thanks. Looks good to me