Although we first thought it would consist in Move Boolean to boolean in the client, exploration showed that we need to stick to Boolean instead:
Query's boolean attributes can either be unset, true or false
The default for each attribute is not something we can rely on in the library (although sometimes it's mentioned in the docstring)
Consequently, we need our boolean getters to return either of three values: Boolean.TRUE, Boolean.FALSE, and null if unset.
This means getters should be implemented as public @Nullable Boolean getXXX().
Likewise, the user should be able to unset an attribute's value, putting the Query in its default state for that attribute.
This means setters should be implemented as public @NonNull Query setXXX(@Nullable Boolean xxx);.
This is implemented in 530c577, which refactors setters that were not compliant with this reasoning; and 68a955c, which adds appropriate annotations.
Boolean attributes testing
Testing of our boolean attributes was inconsistent, sometimes testing TRUE and FALSE cases and sometimes only one of them. Adding a test for the null case was an opportunity to refactor those.
I went the following way:
Extract the Boolean testing logic into testBooleanAttribute, a generic method testing all cases of Boolean values for a given attribute
Refactor Boolean tests to always use testBooleanAttribute. This makes the tests much more concise, more consistent, without any significant performance drawback (runnning 5 times QueryTest before and after this change results in a mean difference of 0.12s).
Reorganize tests into regions for attribute types. If we like the benefits of this new structure, I would apply it to each other attribute type.
For mixed type attributes that can be Boolean (currently ignorePlurals), add assertions for the null case and keep it unchanged until further refactoring.
Boolean attributes handling
Although we first thought it would consist in
Move Boolean to boolean in the client, exploration showed that we need to stick toBooleaninstead:Boolean.TRUE,Boolean.FALSE, andnullif unset.This means getters should be implemented as
public @Nullable Boolean getXXX().Likewise, the user should be able to unset an attribute's value, putting the
Queryin its default state for that attribute.This means setters should be implemented as
public @NonNull Query setXXX(@Nullable Boolean xxx);.This is implemented in 530c577, which refactors setters that were not compliant with this reasoning; and 68a955c, which adds appropriate annotations.
Boolean attributes testing
Testing of our boolean attributes was inconsistent, sometimes testing
TRUEandFALSEcases and sometimes only one of them. Adding a test for thenullcase was an opportunity to refactor those.I went the following way:
testBooleanAttribute, a generic method testing all cases of Boolean values for a given attributetestBooleanAttribute. This makes the tests much more concise, more consistent, without any significant performance drawback (runnning 5 timesQueryTestbefore and after this change results in a mean difference of0.12s).ignorePlurals), add assertions for thenullcase and keep it unchanged until further refactoring.